bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii <eliz@gnu.org>]

> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 31 Dec 2019 10:57:55 +0100
> 
> Now hit any other key.  This sould shrink the echo area and redisplay
> the *scratch* window as before but "extends" the vertical scroll bar
> into the mode line.
> 
> The reason for this behavior is that the vertical scroll bar height in
> the first case is calculated from a window_box_height call where
> ml_row->mode_line_p is set and thus ml_row->height is used.
> 
> In the second case ml_row->mode_line_p is not set and the value is
> taken via estimate_mode_line_height which, given the specification
> from 'test-mode-line', doesn't estimate the height well.

AFAICT, when the ml_row->mode_line_p flag is reset, ml_row->height is
also zero, so disregarding the flag would not have helped us in this
case.

> For the record: Rewriting 'window_box_height' as
> 
> int
> window_box_height (struct window *w)
> {
>    return WINDOW_BOX_TEXT_HEIGHT (w);
> }
> 
> fixes the problem here.

Thanks, but that's not an idea I would like entertaining.  As much as
window_box_text_height and WINDOW_BOX_TEXT_HEIGHT look similar,
there's a subtle difference between them.  The latter is supposed not
to be called during redisplay, because window's mode_line_height field
is not guaranteed to be accurate then, and the fallback is the same
estimate_mode_line_height which doesn't support non-character display
elements.  It is generally okay to use WINDOW_BOX_TEXT_HEIGHT in
window.c, because that code runs outside of redisplay, and the window
object should be up-to-date.  But window_box_text_height is called
_a_lot_ from within redisplay itself.  And here we get to another
subtlety: window_box_text_height uses the window's current_matrix,
which might not be up-to-date yet, depending on where it is called,
because we only update it in update_frame, and set_vertical_scroll_bar
is called long before that.

When a window is resized, as happens in this scenario when the
echo-area message is cleared, the mode-line row changes to a different
one (because it is always the last row of the window), and the new row
didn't yet get updated.  We have the correct data in the
desired_matrix, but not in the current_matrix.  I considered using the
desired_matrix in this case, but that is also somewhat scary: the
desired matrix could be very sparse under some redisplay
optimizations, so we'd need some logic to verify the data is valid
before using it.  Eventually, a simpler solution is just to fall back
to the window's mode_line_height field, before falling back to
estimate_mode_line_height, because when the mode-line height changes,
we schedule an immediate thorough redisplay of the window, and
invalidate that window's mode_line_height field, to be recomputed by
the rescheduled redisplay.  See the proposed patch below.

diff --git a/src/xdisp.c b/src/xdisp.c
index 6b677b63ae..fab95076fe 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -1105,6 +1105,8 @@ window_box_height (struct window *w)
 	   : 0);
       if (ml_row && ml_row->mode_line_p)
 	height -= ml_row->height;
+      else if (w->mode_line_height >= 0)
+	height -= w->mode_line_height;
       else
 	height -= estimate_mode_line_height (f, CURRENT_MODE_LINE_FACE_ID (w));
     }
@@ -1117,6 +1119,8 @@ window_box_height (struct window *w)
 	   : 0);
       if (tl_row && tl_row->mode_line_p)
 	height -= tl_row->height;
+      else if (w->tab_line_height >= 0)
+	height -= w->tab_line_height;
       else
 	height -= estimate_mode_line_height (f, TAB_LINE_FACE_ID);
     }
@@ -1129,6 +1133,8 @@ window_box_height (struct window *w)
 	   : 0);
       if (hl_row && hl_row->mode_line_p)
 	height -= hl_row->height;
+      else if (w->header_line_height >= 0)
+	height -= w->header_line_height;
       else
 	height -= estimate_mode_line_height (f, HEADER_LINE_FACE_ID);
     }