1690296 - Provide API to let apps know when the page changes so that they can display the dynamic toolbar again

Status: VERIFIED FIXED

(GeckoView :: Toolbar, defect, P1)

Description

[Petru-Mugurel Lingurar [:petru]]

From github: https://github.com/mozilla-mobile/fenix/issues/16368.

Steps to reproduce

The following simplified page shows the problem:
https://www.sleeping.pt/wp/testfixedbodybug.html

The following video shows the problem:
https://www.sleeping.pt/wp/wp-content/uploads/2020/10/2020_11_04_14_49_12.webm

1. Scroll down until the toolbar disappears.

2. Click anywhere in the document, to trigger the click event that makes the document fixed.

3. And we can see the space reserved for the toolbar menu but the toolbar is hidden.

When the toolbar reserves the space it does not change the window.innerHeight value either.

Expected behavior

In this case, it is expected that the menu does not reserve space when it is hidden, and the document is set as fixed. Or reserve space but show up and change the window.innerHeight value.

Actual behavior

Reserve the space but don't show up and don't change the window.innerHeight value.

Device information

• Android device: A20e
• Fenix version: 82.1.1

Change performed by the Move to Bugzilla add-on.

Comment 1

[Emily Toop (:fluffyemily)]

:hiro have you any ideas about this one?

Comment 2

[Hiroyuki Ikezoe (:hiro)]

To fix this issue, we need a way to tell browser apps (android-component or Fenix, etc.) to make the toolbar visible again since the content height is no longer taller than the ICB (i.e. screen height - toolbar height).

Comment 3

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)

To fix this issue, we need a way to tell browser apps (android-component or Fenix, etc.) to make the toolbar visible again since the content height is no longer taller than the ICB (i.e. screen height - toolbar height).

Or scroll back a bit.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

I'll see what I can do.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

There require two different patrs; 1) detect the content in question no longer overflowed (during the dynamic toolbar is hidden), 2) notify the detected result to GeckoView

For 1) we can detect it in somewhere around here in ScrollFrameHelper::ReflowFinished with PresShell:GetDynamicToolbarState() check. Maybe we should also handle the visualviewport size change here in PresShell::SetVisualViewportSize.

For 2), the route is PresShell -> BrowserChild -> BrowserParent -> nsWindow -> GeckoSession (-> GeckoDisplay -> GeckoView). We will definitely need to add an (parent part) API in PBrowser.idl for the BrowserChild -> BrowserParent route, for nsWindow -> GeckoSession route this onCompositorAttached would be a reference, I am not sure about the route in Java `GeckoSession -> GeckoDisplay -> GeckoView).

I hope this would help!

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Test HTML

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Bug 1690296: Add callback to notify app to fully display its dynamic toolbar; r=agi!,owlish!,hiro!

• In nsGfxScrollFrame we detect the condition requring the app to expand the toolbar.
  (Hiro, I know that you suggested a second place to detect this. If you feel that it is important enough to add that,
  we'd prefer filing a follow-up bug in Layout for that case that your team can follow up on.)
• We then propagate the notification through PresShell, up through PBrowser, through the nsWindow, then into the GeckoSession
• We invoke a new method on the ContentDelegate. This seemed like the reasonable delegate to use given other existing
  callbacks in the similar vein (such as going fullscreen), but let me know if this should go elsewhere.
• We update GVE and JUnit tests to test this.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ffe5e3633446
Add callback to notify app to fully display its dynamic toolbar; r=agi,owlish,hiro

Comment 11

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/ffe5e3633446

Comment 12

[Petru-Mugurel Lingurar [:petru]]

Thank you for this and sorry for not leaving a message sooner:
We verified on Fenix - https://github.com/mozilla-mobile/fenix/issues/18034#issuecomment-905366382 that the toolbar now appears as expected.

Comment 13

[Chris Peterson [:cpeterson]]

Moving toolbar bugs to the new GeckoView::Toolbar component.