Created attachment 179096 [details] [review]
chat: Make sure we always scroll down when the user enters a message

Calling scrollTo immediately after setting the new text does not always
result into scrolling down. Do this from a style-changed signal handler
instead as this is emitted after the text has been added to the scroll
area.

Review of attachment 179096 [details] [review]:

Thre seem to be a hack in scrollTo that is supposed to deal with that ... will try to fix it instead.

Review of attachment 179096 [details] [review]:

Thre seem to be a hack in scrollTo that is supposed to deal with that ... will try to fix it instead.

Created attachment 179098 [details] [review]
messageTray: Fix scrollTo hack

Instead of immediately setting the adjustment value after the pick
queue a BEFORE_REDRAW priority idle function that does that.

This way the scrolling is done after the new item is added but still
without a visible "jump".

Review of attachment 179098 [details] [review]:

Hmm, this is a bit odd. It's not immediately clear to me why delaying the call to BEFORE_REDRAW helps - basically, if things work normally than the processing sequence would be:

 This dbus callback (at default priority)
 BEFORE_REDRAW idle functions (at BEFORE_REDRAW priority)
 The Clutter master clock (at CLUTTER_PRIORITY_REDRAW)
   - Process events
   - Update animations
   - Compute allocations
   - Repaint

So the allocation won't be computed until after your BEFORE_REDRAW idle executes. The reason that it is actually working is probably the same (to me unknown) reason as Marc-Antoine's patch in bug 614977 - something is forcing a synchronous relayout of the stage in the process of showing a notification.

It's probably good to figure that out, because the right fix here is to force a synchronous relayout *ourselves* - that is, fix up the hack before this code which isn't working - and we'd rather not do two synchronous relayouts. The way I'd determine what is forcing a synchronous relayout is to attach to the process in gdb, break on something that will be called when adding a new notification like st_widget_add_style_class(), trigger a new message being added, then break on _clutter_stage_maybe_relayout() and see what triggered that with a gdb backtrace and gjs_dumpstack().

But leaving that aside, how would we fix up the hack? If I look at the clutter code, I don't see anything in the pick code paths that would cause _clutter_stage_maybe_relayout() to be called - maybe that happened at one point, but currently it just seems to pick against the current (un-updated) allocations. So I think the current hack to call get_actor_at_pos() is completely ineffective. But if you look through the clutter code, something that does force a relayout is clutter_actor_get_allocation_box(). So if you called that on the scrollArea widget, that should force a synchronous stage update, which will update allocation.upper and with luck everything will work.

Well, except for the double synchronous relayout if we can't figure out what is triggering that.

Let's consolidate this with the older bug on the same topic.

*** This bug has been marked as a duplicate of bug 614977 ***