Open Bug 1921331 Opened 1 year ago Updated 2 months ago

[meta] Implement the "overlap model" for the top toolbar

Categories

(Core :: Panning and Zooming, task, P2)

All
Android
task

Tracking

()

People

(Reporter: mstange, Assigned: ajakobi)

References

(Depends on 2 open bugs, Blocks 7 open bugs)

Details

(Keywords: meta)

This is a tracking bug for a rework of the implementation of the Fenix dynamic toolbar, specifically for the toolbar-on-top mode.

Today, in Firefox for Android, if the address bar is on the top (which it is by default), and if toolbars are set to disappear during scrolling (which they are by default), then the GeckoView moves whenever the dynamic toolbar appears or disappears.
Whenever the top toolbar is visible, the GeckoView is pushed down and its bottom is pushed past the bottom of the screen.

The current implementation has a number of disadvantages:

  • When the toolbar shows/hides, it moves the entire website on the screen. (bug 1893287, bug 1893298)
  • Moving the GeckoView causes visual instability (shaky scrolling, elements jerking up and down), because Gecko's compositing is not synced with the shifting of the GeckoView. (bug 1797964, bug 1893225)
  • It affects the physical feel of scroll gestures, due to bugs when dealing with shifting coordinate systems. (bug 1893293)

Furthermore, the current implementation prevents us from fixing bug 1914119: We would like to hide the dynamic toolbar based on scroll notifications from Gecko, rather than by having Fenix inspect touch move events. But if hiding the dynamic toolbar means that the GeckoView is moved upwards, then such an implementation would not work: First, the touch move would be processed by Gecko and result in Gecko scrolling the website up, and then, in response to the scroll position changing, Fenix would move the toolbar and the GeckoView up, so now the website has moved up by twice the intended amount on the screen. But then things would fall apart even more, because now the touch position relative to the GeckoView is the same as before the move, so Gecko would scroll the website back to where it was! But then the dynamic toolbar would move back etc. In other words: bug 1914119 can only work if the page moves relative to the GeckoView. But with the current implementation, the page does not move relative to the GeckoView while the top toolbar is in the process of being hidden.


Instead, I believe we should switch to the "overlap model":

  • The dynamic toolbar-on-top should overlap the GeckoView.
  • The GeckoView itself should never move.

For this to work, we need to implement a few things in APZ-land:

  • When the GeckoView is at 0,0, and the top toolbar is visible, it must still be possible to scroll a website all the way to the top - the top end of the website must be just below the top toolbar. This means that APZ needs to allow "scrolling" / moving the web page far enough down so that there will be a "gap" at the top of the GeckoView. (But this gap is then covered by the toolbar.)
  • APZ needs to decouple the "position of the website in the GeckoView" from the "scroll position".
  • APZ needs to make sure that the "scroll position" which is exposed via the DOM remains "the amount of the page that is hidden beyond the bottom edge of the top toolbar". (This definition assumes there is no pinch zooming; things get a bit more complicated when zoomed in.)
  • If Fenix decides to reveal the top toolbar at any point, APZ needs to correctly handle that doing so will change the "scroll position" that's exposed to the website, without changing the position at which the website is rendered in the GeckoView.
  • Special care needs to be taken when Fenix wants to reveal the toolbar during a navigation: If the toolbar is currently hidden, and you tap a link to navigate to a new page, the top of the new website should not end up covered by the top toolbar.

The following changes are initially out-of-scope for this bug:

  • bug 1914119: Let's assume that the showing/hiding of dynamic toolbars will initially still be controlled by Fenix inspecting touch events, as today. We just wouldn't be moving the GeckoView anymore.
  • bug 1921339: Let's assume that the presence of the bottom toolbar will not affect the scroll range, just like it doesn't today.
  • "Stable scroll position" while the top toolbar is hiding: With the proposed implementation, the moving of the page and the hiding of the toolbar are happening in different places and are not in sync. This can cause fluctuations in the scroll position value that's exposed via the DOM. I think today's implementation has the same problem. We can solve it as a follow-up (potentially by moving more of the dynamic toolbar logic into APZ, and maybe by having a more declarative way for Fenix to tell APZ about the desired dynamic toolbar behavior).

But we may decide later that it's easier to implement all of these things at the same time.

Type: defect → task
Blocks: 1921339
Blocks: 1917726
Blocks: 1917945
Blocks: 1797964
Blocks: 1893225
Priority: -- → P2

This bug doesn't need to directly block bug 1917945 because it already blocks meta bug 1914119, which blocks bug 1917945.

No longer blocks: 1917945

Thinking about this and seeing the We can solve it as a follow-up (potentially by moving more of the dynamic toolbar logic into APZ remark I want to make sure "the Android way", a more coopoerative way is also considered:

  • APZ->GeckoView would inform about the distance scrolled and do nothing else yet
  • AC->Fenix would consume none/some/all off that distance to translate the toolbar and report back to GeckoView->APZ the unconsumed distance (if the toolbar is already hidden/shown and cannot be translated anymore then all of the scroll distance is "unconsumed" by Fenix)
  • APZ uses the unconsumed (by GeckoView integrators) distance to scroll the content.

This framework can then easily support features like the overscroll edge effect or pull to refresh, in any combination without APZ/GeckoView caring about these.

It would mean that with the dynamic toolbar at top
If the user swipes up then GeckoView is not initially scrolling but the entire page moves up and more web content is displayed until the toolbar is hidden after which the webpage content can be scrolled.
Visually (don't know the implementation details) it seems like this is the behaviour on Chrome.

See Also: → 1945945
Assignee: nobody → ajakobi
Blocks: 1880375
Depends on: 1951421
Depends on: 1953509

Thinking about integrating this new model, will there be a new API similar to setVerticalClipping but for the top toolbar also?

Yes, that's the goal - in my prototype in bug 1945945 I renamed setVerticalClipping to setCurrentClipInsets(top, bottom).

Blocks: 1893287
You need to log in before you can comment on or make changes to this bug.