Fix vertical cursor motion when the newline is covered by a display string.

 src/indent.c (Fvertical_motion): Compute and apply the overshoot
 logic when moving up, not only when moving down.  Fix the
 confusing name and values of the it_overshoot_expected variable;
 logic changes accordingly.  (Bug#9254) (Bug#9549)
 src/xdisp.c (pos_visible_p): Produce correct pixel coordinates when
 CHARPOS is covered by a display string which includes newlines.
 (move_it_vertically_backward): Avoid inflooping when START_CHARPOS
 is covered by a display string with embedded newlines.

3 files changed, 233 insertions, 49 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index be80149139f..7c7eaf6c779 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,15 @@
+2011-09-24  Eli Zaretskii  <eliz@gnu.org>
+
+        * indent.c (Fvertical_motion): Compute and apply the overshoot
+        logic when moving up, not only when moving down.  Fix the
+        confusing name and values of the it_overshoot_expected variable;
+        logic changes accordingly.  (Bug#9254) (Bug#9549)
+
+        * xdisp.c (pos_visible_p): Produce correct pixel coordinates when
+        CHARPOS is covered by a display string which includes newlines.
+        (move_it_vertically_backward): Avoid inflooping when START_CHARPOS
+        is covered by a display string with embedded newlines.
+
 2011-09-24  Michael Albinus  <michael.albinus@gmx.de>
 
         * dbusbind.c (Fdbus_register_signal): Add match rule to
diff --git a/src/indent.c b/src/indent.c
index e00d2152577..e6629ef5811 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -2019,7 +2019,8 @@ whether or not it is currently displayed in some window.  */)
   else
     {
       EMACS_INT it_start;
-      int first_x, it_overshoot_expected IF_LINT (= 0);
+      int first_x, it_overshoot_count = 0;
+      int overshoot_handled = 0;
 
       itdata = bidi_shelve_cache ();
       SET_TEXT_POS (pt, PT, PT_BYTE);
@@ -2028,22 +2029,23 @@ whether or not it is currently displayed in some window.  */)
       it_start = IT_CHARPOS (it);
 
       /* See comments below for why we calculate this.  */
-      if (XINT (lines) > 0)
+      if (it.cmp_it.id >= 0)
+        it_overshoot_count = 0;
+      else if (it.method == GET_FROM_STRING)
         {
-          if (it.cmp_it.id >= 0)
-            it_overshoot_expected = 1;
-          else if (it.method == GET_FROM_STRING)
+          const char *s = SSDATA (it.string);
+          const char *e = s + SBYTES (it.string);
+          while (s < e)
             {
-              const char *s = SSDATA (it.string);
-              const char *e = s + SBYTES (it.string);
-              while (s < e && *s != '\n')
-                ++s;
-              it_overshoot_expected = (s == e) ? -1 : 0;
+              if (*s++ == '\n')
+                it_overshoot_count++;
             }
-          else
-            it_overshoot_expected = (it.method == GET_FROM_IMAGE
-                                     || it.method == GET_FROM_STRETCH);
+          if (!it_overshoot_count)
+            it_overshoot_count == -1;
         }
+      else
+        it_overshoot_count =
+          !(it.method == GET_FROM_IMAGE || it.method == GET_FROM_STRETCH);
 
       /* Scan from the start of the line containing PT.  If we don't
          do this, we start moving with IT->current_x == 0, while PT is
@@ -2057,6 +2059,25 @@ whether or not it is currently displayed in some window.  */)
            tell, and it causes Bug#2694 .  -- cyd */
         move_it_to (&it, PT, -1, -1, -1, MOVE_TO_POS);
 
+      /* IT may move too far if truncate-lines is on and PT lies
+         beyond the right margin.  IT may also move too far if the
+         starting point is on a Lisp string that has embedded
+         newlines.  In these cases, backtrack.  */
+      if (IT_CHARPOS (it) > it_start)
+        {
+          /* We need to backtrack also if the Lisp string contains no
+             newlines, but there is a newline right after it.  In this
+             case, IT overshoots if there is an after-string just
+             before the newline.  */
+          if (it_overshoot_count < 0
+              && it.method == GET_FROM_BUFFER
+              && it.c == '\n')
+            it_overshoot_count = 1;
+          if (it_overshoot_count > 0)
+            move_it_by_lines (&it, -it_overshoot_count);
+
+          overshoot_handled = 1;
+        }
       if (XINT (lines) <= 0)
         {
           it.vpos = 0;
@@ -2065,47 +2086,31 @@ whether or not it is currently displayed in some window.  */)
           if (XINT (lines) == 0 || IT_CHARPOS (it) > 0)
             move_it_by_lines (&it, max (INT_MIN, XINT (lines)));
         }
+      else if (overshoot_handled)
+        {
+          it.vpos = 0;
+          move_it_by_lines (&it, min (INT_MAX, XINT (lines)));
+        }
       else
         {
-          if (IT_CHARPOS (it) > it_start)
-            {
-              /* IT may move too far if truncate-lines is on and PT
-                 lies beyond the right margin.  In that case,
-                 backtrack unless the starting point is on an image,
-                 stretch glyph, composition, or Lisp string.  */
-              if (!it_overshoot_expected
-                  /* Also, backtrack if the Lisp string contains no
-                     newline, but there is a newline right after it.
-                     In this case, IT overshoots if there is an
-                     after-string just before the newline.  */
-                  || (it_overshoot_expected < 0
-                      && it.method == GET_FROM_BUFFER
-                      && it.c == '\n'))
-                move_it_by_lines (&it, -1);
-              it.vpos = 0;
-              move_it_by_lines (&it, min (INT_MAX, XINT (lines)));
-            }
-          else
+          /* Otherwise, we are at the first row occupied by PT, which
+             might span multiple screen lines (e.g., if it's on a
+             multi-line display string).  We want to start from the
+             last line that it occupies.  */
+          if (it_start < ZV)
             {
-              /* Otherwise, we are at the first row occupied by PT,
-                 which might span multiple screen lines (e.g., if it's
-                 on a multi-line display string).  We want to start
-                 from the last line that it occupies.  */
-              if (it_start < ZV)
-                {
-                  while (IT_CHARPOS (it) <= it_start)
-                    {
-                      it.vpos = 0;
-                      move_it_by_lines (&it, 1);
-                    }
-                  if (XINT (lines) > 1)
-                    move_it_by_lines (&it, min (INT_MAX, XINT (lines) - 1));
-                }
-              else
+              while (IT_CHARPOS (it) <= it_start)
                 {
                   it.vpos = 0;
-                  move_it_by_lines (&it, min (INT_MAX, XINT (lines)));
+                  move_it_by_lines (&it, 1);
                 }
+              if (XINT (lines) > 1)
+                move_it_by_lines (&it, min (INT_MAX, XINT (lines) - 1));
+            }
+          else
+            {
+              it.vpos = 0;
+              move_it_by_lines (&it, min (INT_MAX, XINT (lines)));
             }
         }
 
diff --git a/src/xdisp.c b/src/xdisp.c
index 0d870671c69..3f545fae248 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -1210,6 +1210,34 @@ line_bottom_y (struct it *it)
   return line_top_y + line_height;
 }
 
+/* Subroutine of pos_visible_p below.  Extracts a display string, if
+   any, from the display spec given as its argument.  */
+static Lisp_Object
+string_from_display_spec (Lisp_Object spec)
+{
+  if (CONSP (spec))
+    {
+      while (CONSP (spec))
+        {
+          if (STRINGP (XCAR (spec)))
+            return XCAR (spec);
+          spec = XCDR (spec);
+        }
+    }
+  else if (VECTORP (spec))
+    {
+      int i;
+
+      for (i = 0; i < ASIZE (spec); i++)
+        {
+          if (STRINGP (AREF (spec, i)))
+            return AREF (spec, i);
+        }
+      return Qnil;
+    }
+
+  return spec;
+}
 
 /* Return 1 if position CHARPOS is visible in window W.
    CHARPOS < 0 means return info about WINDOW_END position.
@@ -1304,6 +1332,136 @@ pos_visible_p (struct window *w, EMACS_INT charpos, int *x, int *y,
                     }
                 }
             }
+          else if (IT_CHARPOS (it) != charpos)
+            {
+              Lisp_Object cpos = make_number (charpos);
+              Lisp_Object spec = Fget_char_property (cpos, Qdisplay, Qnil);
+              Lisp_Object string = string_from_display_spec (spec);
+              int newline_in_string = 0;
+
+              if (STRINGP (string))
+                {
+                  const char *s = SSDATA (string);
+                  const char *e = s + SBYTES (string);
+                  while (s < e)
+                    {
+                      if (*s++ == '\n')
+                        {
+                          newline_in_string = 1;
+                          break;
+                        }
+                    }
+                }
+              /* The tricky code below is needed because there's a
+                 discrepancy between move_it_to and how we set cursor
+                 when the display line ends in a newline from a
+                 display string.  move_it_to will stop _after_ such
+                 display strings, whereas set_cursor_from_row
+                 conspires with cursor_row_p to place the cursor on
+                 the first glyph produced from the display string.  */
+
+              /* We have overshoot PT because it is covered by a
+                 display property whose value is a string.  If the
+                 string includes embedded newlines, we are also in the
+                 wrong display line.  Backtrack to the correct line,
+                 where the display string begins.  */
+              if (newline_in_string)
+                {
+                  Lisp_Object startpos, endpos;
+                  EMACS_INT start, end;
+                  struct it it3;
+
+                  /* Find the first and the last buffer positions
+                     covered by the display string.  */
+                  endpos =
+                    Fnext_single_char_property_change (cpos, Qdisplay,
+                                                       Qnil, Qnil);
+                  startpos =
+                    Fprevious_single_char_property_change (endpos, Qdisplay,
+                                                           Qnil, Qnil);
+                  start = XFASTINT (startpos);
+                  end = XFASTINT (endpos);
+                  /* Move to the last buffer position before the
+                     display property.  */
+                  start_display (&it3, w, top);
+                  move_it_to (&it3, start - 1, -1, -1, -1, MOVE_TO_POS);
+                  /* Move forward one more line if the position before
+                     the display string is a newline or if it is the
+                     rightmost character on a line that is
+                     continued or word-wrapped.  */
+                  if (it3.method == GET_FROM_BUFFER
+                      && it3.c == '\n')
+                    move_it_by_lines (&it3, 1);
+                  else if (move_it_in_display_line_to (&it3, -1,
+                                                       it3.current_x
+                                                       + it3.pixel_width,
+                                                       MOVE_TO_X)
+                           == MOVE_LINE_CONTINUED)
+                    {
+                      move_it_by_lines (&it3, 1);
+                      /* When we are under word-wrap, the #$@%!
+                         move_it_by_lines moves 2 lines, so we need to
+                         fix that up.  */
+                      if (it3.line_wrap == WORD_WRAP)
+                        move_it_by_lines (&it3, -1);
+                    }
+
+                  /* Record the vertical coordinate of the display
+                     line where we wound up.  */
+                  top_y = it3.current_y;
+                  if (it3.bidi_p)
+                    {
+                      /* When characters are reordered for display,
+                         the character displayed to the left of the
+                         display string could be _after_ the display
+                         property in the logical order.  Use the
+                         smallest vertical position of these two.  */
+                      start_display (&it3, w, top);
+                      move_it_to (&it3, end + 1, -1, -1, -1, MOVE_TO_POS);
+                      if (it3.current_y < top_y)
+                        top_y = it3.current_y;
+                    }
+                  /* Move from the top of the window to the beginning
+                     of the display line where the display string
+                     begins.  */
+                  start_display (&it3, w, top);
+                  move_it_to (&it3, -1, 0, top_y, -1, MOVE_TO_X | MOVE_TO_Y);
+                  /* Finally, advance the iterator until we hit the
+                     first display element whose character position is
+                     CHARPOS, or until the first newline from the
+                     display string, which signals the end of the
+                     display line.  */
+                  while (get_next_display_element (&it3))
+                    {
+                      PRODUCE_GLYPHS (&it3);
+                      if (IT_CHARPOS (it3) == charpos
+                          || ITERATOR_AT_END_OF_LINE_P (&it3))
+                        break;
+                      set_iterator_to_next (&it3, 0);
+                    }
+                  top_x = it3.current_x - it3.pixel_width;
+                  /* Normally, we would exit the above loop because we
+                     found the display element whose character
+                     position is CHARPOS.  For the contingency that we
+                     didn't, and stopped at the first newline from the
+                     display string, move back over the glyphs
+                     prfoduced from the string, until we find the
+                     rightmost glyph not from the string.  */
+                  if (IT_CHARPOS (it3) != charpos && EQ (it3.object, string))
+                    {
+                      struct glyph *g = it3.glyph_row->glyphs[TEXT_AREA]
+                                        + it3.glyph_row->used[TEXT_AREA];
+
+                      while (EQ ((g - 1)->object, string))
+                        {
+                          --g;
+                          top_x -= g->pixel_width;
+                        }
+                      xassert (g < it3.glyph_row->glyphs[TEXT_AREA]
+                                    + it3.glyph_row->used[TEXT_AREA]);
+                    }
+                }
+            }
 
           *x = top_x;
           *y = max (top_y + max (0, it.max_ascent - it.ascent), window_top_y);
@@ -8521,7 +8679,16 @@ move_it_vertically_backward (struct it *it, int dy)
       move_it_to (&it2, start_pos, -1, -1, it2.vpos + 1,
                   MOVE_TO_POS | MOVE_TO_VPOS);
     }
-  while (!IT_POS_VALID_AFTER_MOVE_P (&it2));
+  while (!(IT_POS_VALID_AFTER_MOVE_P (&it2)
+           /* If we are in a display string which starts at START_POS,
+              and that display string includes a newline, and we are
+              right after that newline (i.e. at the beginning of a
+              display line), exit the loop, because otherwise we will
+              infloop, since move_it_to will see that it is already at
+              START_POS and will not move.  */
+           || (it2.method == GET_FROM_STRING
+               && IT_CHARPOS (it2) == start_pos
+               && SREF (it2.string, IT_STRING_BYTEPOS (it2) - 1) == '\n')));
   xassert (IT_CHARPOS (*it) >= BEGV);
   SAVE_IT (it3, it2, it3data);