bug#18545: 24.4.50: Bug - forward-line inside with-selected-window

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sun, 28 Sep 2014 19:29:47 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: lompik@voila.fr, 18545@debbugs.gnu.org
> 
> > Date: Sun, 28 Sep 2014 11:34:44 +0200
> > From: martin rudalics <rudalics@gmx.at>
> > CC: lompik@voila.fr, 18545@debbugs.gnu.org
> > 
> >  > Hmm... I wonder why did we enter this area of the code, i.e. why did
> >  > this condition fire:
> >  >
> >  >    /* Handle case where place to start displaying has been specified,
> >  >       unless the specified location is outside the accessible range.  */
> >  >    if (w->force_start || window_frozen_p (w))
> >  >
> >  > Was w->force_start non-zero, or did window_frozen_p return non-zero?
> >  > If the former, can you see where was the force_start flag set?  (I'd
> >  > be surprised if it's the latter, since windows are only frozen when we
> >  > grow the minibuffer, which shouldn't be happening here, I think.)
> >  >
> >  > In general, if the force_start flag of a window is set, that means we
> >  > shouldn't scroll, so bringing point back into view makes sense.
> > 
> > That's likely me.  Under certain conditions I do `recenter' with a
> > negative argument in `post-command-hook' which basically looks like
> > 
> >               (recenter (max 0 (- (window-height) 7)))
> > 
> > which triggers w->force_start being set here in xdisp.c:
> > 
> >        if (IT_CHARPOS (it) == PT)
> > 	w->force_start = 1;
> 
> That's what I thought.
> 
> > So my case _is_ very likely special.  Still "bringing point back into
> > view" shouldn't make point appear on a partially visible line.
> 
> No, that's likely the bug here.  I will look into that.

Does the patch below help?  If not, can you tell where I goofed?

=== modified file 'src/xdisp.c'
--- src/xdisp.c	2014-09-25 07:01:35 +0000
+++ src/xdisp.c	2014-09-28 17:45:22 +0000
@@ -16179,15 +16179,21 @@ redisplay_window (Lisp_Object window, bo
       && CHARPOS (startp) >= BEGV
       && CHARPOS (startp) <= ZV)
     {
+      ptrdiff_t it_charpos;
+
       w->optional_new_start = 0;
       start_display (&it, w, startp);
       move_it_to (&it, PT, 0, it.last_visible_y, -1,
 		  MOVE_TO_POS | MOVE_TO_X | MOVE_TO_Y);
-      if (IT_CHARPOS (it) == PT)
-	w->force_start = 1;
-      /* IT may overshoot PT if text at PT is invisible.  */
-      else if (IT_CHARPOS (it) > PT && CHARPOS (startp) <= PT)
-	w->force_start = 1;
+      it_charpos = IT_CHARPOS (it);
+      if (line_bottom_y (&it) < it.last_visible_y)
+	{
+	  if (it_charpos == PT)
+	    w->force_start = 1;
+	  /* IT may overshoot PT if text at PT is invisible.  */
+	  else if (it_charpos > PT && CHARPOS (startp) <= PT)
+	    w->force_start = 1;
+	}
     }
 
  force_start: