1740289 - onShowDynamicToolbar does not fire though the webpage is updated

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Petru-Mugurel Lingurar [:petru]]

Attachment: onShowDynamicToolbarNotBeingCalled.mp4

Issue seen while testing https://github.com/mozilla-mobile/fenix/issues/9970

#1690296 added a new ContentDelegate#onShowDynamicToolbar method to inform clients when the toolbar should be shown.
Tested now and confirmed that callback fires for the reddit STRs.

But when testing on gv-sandbox.glitch.me whether a previously hidden toolbar appears when the content is set to not be scrollable I see the webpage content updated but the onShowDynamicToolbar is not firing.

Not sure whether this callback should fire in the current implementation but if not maybe the functionality can be expanded to help in this case also?

Comment 1

[Emily Toop (:fluffyemily)]

This seems to be about correctly detecting whether the page is still scrollable. Can someone please take a look for us?

Comment 2

[Emily McDonough [:alaskanemily]]

Hiro, can you take a look at this?

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Yeah, indeed this case is a fell out case in bug 1690296. An excuse I can say is overflow: hidden is scrollable in terms of the spec. The fix should be straightforward.

I am taking for now, that said, if one of the new GeckoView team members is looking for an easy-to-handle bug, I'd be happy to mentor the person. If so please let me know.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1740289 - Notify onShowDynamicToolbar on overflow: hidden case. r?#layout-reviewers

The reason why we don't change GetOverflowState is that GetOverflowState is also
used for overflow/underflow event firing and our frontend code expects both
the events are fired on overflow:hidden elements [1]. And it looks like it's by
design as per one of test cases for overflow events has overflow:hidden style
in the first place.

[1] https://searchfox.org/mozilla-central/rev/df6434d2ebfdf2b5f89f205fc81d60d64a774fe1/devtools/client/shared/components/tabs/Tabs.css#38-39
[2] https://searchfox.org/mozilla-central/rev/df6434d2ebfdf2b5f89f205fc81d60d64a774fe1/layout/generic/test/test_overflow_event.html#46

Comment 5

[Petru-Mugurel Lingurar [:petru]]

Another related issue which could be resolved by these changes - https://github.com/mozilla-mobile/fenix/issues/22441

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6477cdcc812e
Notify onShowDynamicToolbar on overflow: hidden case. r=geckoview-reviewers,agi,emilio

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/6477cdcc812e