1991654 - Use scroll data from GeckoView for animating toolbar

Status: RESOLVED FIXED

(Firefox for Android :: Toolbar, task, P2)

Description

[Petru-Mugurel Lingurar [:petru]]

This should use the data bug 1990215 makes available to animate the bottom toolbar.
Special care is needed to also snap the toolbar to expanded or collapsed when the scroll ends to avoid leaving the toolbar shown in half.

Comment 1

[Petru-Mugurel Lingurar [:petru]]

Attachment: InitialResultsInFenix.mp4

@Botond Seems like the implementation in Fenix is easier than first thought and I already got a quick POC.
Seeing though a scenario that should probably be avoided in reporting scroll updates: the webpage being zoomed in/out.
Currently we infer such gestures and avoid scrolling the toolbar, seems like Chrome is doing the same (or was, their dynamic toolbar seems broken in recent releases).
I think this information is available to APZ so could it be used to avoid sending scroll updates when the page actually being zoomed in/out?

Comment 2

[Botond Ballo [:botond]]

(In reply to Petru-Mugurel Lingurar [:petru] from comment #1)

@Botond Seems like the implementation in Fenix is easier than first thought and I already got a quick POC.

Nice!

Seeing though a scenario that should probably be avoided in reporting scroll updates: the webpage being zoomed in/out.
Currently we infer such gestures and avoid scrolling the toolbar, seems like Chrome is doing the same (or was, their dynamic toolbar seems broken in recent releases).
I think this information is available to APZ so could it be used to avoid sending scroll updates when the page actually being zoomed in/out?

Since the object passed as argument to the CompositorScrollDelegate, ScrollPositionUpdate, already has a zoom field, could the consumer of the delegate already accomplish this by checking whether the zoom has changed since the last update, and not moving the toolbar in response to the update in that case?

Comment 3

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1991654 - part 1 - Rename current scrolling behavior to EngineViewScrollingGesturesBehavior r=#android-reviewers

The plan is to use the previous name - EngineViewScrollingBehavior for the supertype
of this and also of a new behavior using the scroll data for animating the toolbar.

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1991654 - part 2 - Use a generic EngineViewScrollingBehavior r=#android-reviewers

This will allow to easily switch between the current way of scrolling
the dynamic toolbar and the new one based on APZ scrolling data.

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1991654 - part 3 - Use new EngineViewScrollingDataBehavior r=#android-reviewers

This will animate the bottom toolbar based on scroll data
exposed by GeckoView instead of inferring the scroll distances
based on intercepted MotionEvents.
As a result the dynamic toolbar will be always kept in sync
with the engine view being scrolled.

Comment 6

[Petru-Mugurel Lingurar [:petru]]

(In reply to Botond Ballo [:botond] from comment #2)

Since the object passed as argument to the CompositorScrollDelegate, ScrollPositionUpdate, already has a zoom field, could the consumer of the delegate already accomplish this by checking whether the zoom has changed since the last update, and not moving the toolbar in response to the update in that case?

Thank you!
Works better than I expected 😅.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/06b4ab52c3eb
https://hg.mozilla.org/integration/autoland/rev/1a76a2775d56
part 1 - Rename current scrolling behavior to EngineViewScrollingGesturesBehavior r=android-reviewers,skhan
https://github.com/mozilla-firefox/firefox/commit/ad7bf747a5d4
https://hg.mozilla.org/integration/autoland/rev/9ce957c40272
part 2 - Use a generic EngineViewScrollingBehavior r=android-reviewers,Roger
https://github.com/mozilla-firefox/firefox/commit/a13db57fbf6f
https://hg.mozilla.org/integration/autoland/rev/a7417af900ff
part 3 - Use new EngineViewScrollingDataBehavior r=android-reviewers,Roger

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/1a76a2775d56
https://hg.mozilla.org/mozilla-central/rev/9ce957c40272
https://hg.mozilla.org/mozilla-central/rev/a7417af900ff