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

[martin rudalics]

> 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.

Right, I've been there initially.

>> 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.

Neither do I.

> As much as
> window_box_text_height

"window_box_height" presumably, also everywhere below.

> 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,

Your patch does use it though.

> 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,

window.c doesn't use it.  It's used in xdisp.c only.  And I'm sure it
can return negative values, sometimes.  So I presume we can do away
with it then?

> 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.

I've tried via

          w->mode_line_height
            = MATRIX_MODE_LINE_ROW (w->current_matrix)->height
            = DESIRED_MODE_LINE_HEIGHT (w);

after displaying the mode lines without great success (likely because
window_box_height didn't go for it then).

> 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.

This fails here when loading the attached test-popup-2.el and typing,
for example, F2 <up> F3 <up> F2 <up>.

martin

test-popup-2.el
Description: Text document