[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

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

Simon McVittie <smcv@debian.org> (2021-05-23):
> > > 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 was willing to have that for buster, given we're planning on
rewriting the whole thing for GTK 3 next release cycle away. But indeed,
that's quite messy, keeping track of other things we shouldn't be caring
about. And as mentioned, we are a few events behind anyway.

> 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

I'll get back to that in a minute…

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

I've also toyed with the idea of sending a specific “kibi-event” from
the set_text_in_idle that would have its dedicated, not one-shot
callback (focus_path disconnects itself) on the TreeView side, but that
one also comes in too early.

And right before getting some rest, it struck me that I should be
looking into the size-request and size-allocate signals. The latter is
the right one and that's indeed getting me the focus where it should be!
\o/ I suppose this isn't entirely crazy since that's an idea you had as
well. :)

… back to MR #12, I'm a little reluctant to changing the workaround at
this stage. After all, I've run a bunch of test cases already, which all
looked good, and while reverting the initial workaround would mean less
code / technical debt, all those lines should go away in bookworm
anyway. So I think I would *slightly* prefer paper over the focus
regression (this bug) with an extra callback. I would have to need to
power up coffee and brain a little more though. :)

Thanks for all your feedback, much appreciated as always.

Cyril Brulebois (kibi@debian.org)            <https://debamax.com/>
D-I release manager -- Release team member -- Freelance Consultant

Attachment: signature.asc
Description: PGP signature

Reply to: