Bug#988951: regression: focus_path on last items no longer works properly

[Simon McVittie]

On Sun, 23 May 2021 at 11:31:58 +0100, Simon McVittie wrote:
> >  - Slightly shorter (`kvm -m 1G -cdrom mini.iso`, no disk layout or even
> >    disk required), pick a language like French and all default choices,
> >    until the mirror country selection, pick the very last one
> >    (États-Unis), and on the mirror host selection, pick the very last
> >    one again (the actual hostname doesn't matter). Now, on the next
> >    dialog, hit “Revenir en arrière” (Back), and see the selected
> >    hostname isn't focussed. Another step back shows the selected country
> >    isn't focussed either. That should happen with other languages as
> >    well, using French has the main advantage for me to get the
> >    appropriate keyboard layout automatically plus get two “back” steps
> >    that exhibit the problem (other countries might not have a mirror
> >    list as big as the US one).
> 
> I can read French somewhat better than Swedish (and infinitely better
> than Sinhala where I don't even recognise the glyphs), so this is probably
> the more convenient test-case :-)

I can reproduce something similar with more defaults, like this:

* accept defaults (in particular en_US) until you reach the list of
  USA mirrors
* choose the last mirror
* the next question asks for a proxy; do not answer, but instead "Go back"
* good result: the mirror you chose is focused and visible
* bad result: the mirror you chose is focused but you have to scroll with
  the mouse to be able to see that

> > I suppose we have a slightly taller widget at
> > first, we scroll down to the bottom; then when set_text_in_idle happens,
> > the widget is resized slightly smaller, the position is not correct
> > anymore (it's no longer “full-bottom” but a little higher as seen in the
> > scrollbar), and the selected line gets out of sight.

I think you're correct. Deferring addition of the text to the GtkTextView
means the initial window layout for select/multiselect questions will
be done based on the assumption that there is no text, which means the
GtkTreeView will receive nearly the full window height. When we scroll
the GtkTreeView to bring the selected row into view, that also happens
before the text is added (I think we probably draw one frame without the
text, then add the text in the next frame).

When the GtkTreeView is resized as a result of the text being added,
the top left corner of the visible area is what's preserved; if its
selected row was near the bottom, the result is that the selected row
is no longer visible.

> > I've tried various things like having the focus_path happens in a
> > “_later” indirection using the same kind of logic as Simon introduced
> > for setting the text (with a different priority), but that would happen
> > waaay before set_text_in_idle anyway.
> 
> Please could you share what you tried so I can check whether it's doing
> something wrong? I feel as though this approach ought to be workable

Actually, never mind: the code structure for this gets increasingly messy,
with components needing to know about implementation details of unrelated
components. I think that's technical debt we probably don't want to pay.

I have a couple of ideas for possible ways to deal with this.

One idea is to undo my workaround for #988787 on the cdebconf side,
and instead, hook onto the GtkTextView's size-request signal and force
it to be allocated with at least some arbitrary reasonable size (I'm
trying 300px). This will hopefully still work around #988787, and if
it doesn't, we have the workaround #988786 in GTK as a fallback.
https://salsa.debian.org/installer-team/cdebconf/-/merge_requests/12

Another idea, still in cdebconf, is to connect to whatever signal is
emitted when the GtkTreeView is resized (I think this is size-allocate?),
and take that opportunity to re-adjust the scroll position so the (first)
selected row comes into view. However, I'm a bit concerned that this
could itself cause an infinite loop like #988787 - I'd have to check
the GtkTreeView implementation to make sure scrolling cannot schedule
a resize.

    smcv