In 1516048 setVerticalClipping was added which allows us to adjusts bottom aligned content with our moving toolbar. When the bottom content is adjusted, it cannot be interacted with.

STR:

1. Visit site with bottom bar (ex sozone.de) on Fenix/RB
2. Try to disimss pushed up cookie banner

Expected: Can dismiss
Actual: Cannot dismiss until toolbar is collapsed

See: https://github.com/mozilla-mobile/fenix/issues/2574

Can reproduce with Fenix and RB.

Adding [geckoview:fenix:m5] whiteboard tag because we need to fix this dynamic toolbar bug for Fenix MVP.

Adding [geckoview:fenix:p2] whiteboard tag and 68=wontfix. We don't need to fix this for Fenix MVP because MVP will use a fixed-position bottom address bar.

This is a dynamic address bar problem.

Fenix issue: https://github.com/mozilla-mobile/fenix/issues/2574

I'm editing a bunch of GeckoView bugs. If you'd like to filter all this bugmail, search and destroy emails containing this UUID:

e88a5094-0fc0-4b7c-b7c5-aef00a11dbc9

We need this API so Fenix can fix bottom nav bar issue https://github.com/mozilla-mobile/fenix/issues/2574.

snorp is going to talk to Jessie and then assign this bug to Botond or himself.

Tentatively assigning to Botond because snorp says he's going to look at this bottom toolbar bug soon.

This bug is still P1 because Fenix's dynamic bottom nav bar depends on this bug.

Note that to investigate this I'll need to build Fenix against a custom GeckoView. Are there instructions for how to do that?

Adding this bug to GV's September sprint.

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

Note that to investigate this I'll need to build Fenix against a custom GeckoView. Are there instructions for how to do that?

https://mozilla.github.io/geckoview/contributor/geckoview-quick-start#include-geckoview-as-a-dependency

How can I enable the dynamic toolbar in Fenix to reproduce this?

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

How can I enable the dynamic toolbar in Fenix to reproduce this?

Sebastian? ^

Looks like this has been answered on Slack already:
https://github.com/csadilek/fenix/commit/b48310092ac7a56ab51930356879929fdb024444

Another option would be to just use Reference Browser where this is enabled by default for all installations.

(In reply to Emily Kager [:ekager] from comment #0)

In 1516048 setVerticalClipping was added which allows us to adjusts bottom aligned content with our moving toolbar. When the bottom content is adjusted, it cannot be interacted with.

STR:

1. Visit site with bottom bar (ex sozone.de) on Fenix/RB
2. Try to disimss pushed up cookie banner

Expected: Can dismiss
Actual: Cannot dismiss until toolbar is collapsed

I tested this in Reference Browser. The first thing I'm noticing is that the "pushing up" behaviour doesn't happen. The position of the bottom bar doesn't seem to be adjusted, rather it's partially obscured by the toolbar.

It's not clear yet whether that's because we've regressed the behaviour of the setVerticalClipping API in the platform, or if R-B stopped using that API.

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

It's not clear yet whether that's because we've regressed the behaviour of the setVerticalClipping API in the platform, or if R-B stopped using that API.

It's the latter: R-B is not calling setVerticalClipping.

Sebastian, are you aware of a change related to this in R-B or Android Components since this bug was filed?

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

It's the latter: R-B is not calling setVerticalClipping.

The same is true of Fenix with this patch applied to enable the dynamic toolbar.

Tangentially related, but on the platform side setVerticalClipping will need a WebRender implementation as well; filed bug 1583380 for this.

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

R-B is not calling setVerticalClipping.

Snorp suggested that I work around this locally by hacking in a call to setVerticalClipping around here. I did so, and I can confirm that I can now reproduce this bug as originally reported.

I filed an A-C bug about Fenix and R-B not calling setVerticalClipping.

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

Snorp suggested that I work around this locally by hacking in a call to setVerticalClipping around here. I did so, and I can confirm that I can now reproduce this bug as originally reported.

The problem here is that the implementation of setVerticalClipping added in bug 1516048 is only adjusting the position of the bottom-fixed content for the purposes of rendering. We need to make a corresponding adjustment for purposes of hit testing.

This is not going to be a trivial fix.

In Fennec, the async transforms introduced by the dynamic toolbar were transient -- they would go away as soon as we repaint. As a result, we obeyed them during rendering so that dynamic toolbar transitions would be smooth, but we ignored them during hit testing because you were unlikely to try to interact with an element during the brief time when we have updated its position visually but have not repainted yet.

Fenix's approach to the dynamic toolbar, however, involves an async transform that's persistent as long as the toolbar is partially or fully on-screen. This means we have to obey it during hit testing, which will require some extra machinery. It will also require support in WebRender, like the rendering part.

Though I might be misunderstanding what the issue is here, what I can tell from the perspective of a web compat issue (bug 1515980) is, regardless of whether the toolbar is static or dynamic, GeckoView needs to set client height which is NOT including the toolbar height to GeckoSession.mHeight and notify the toolbar height to Gecko independently. So that in the case of the dynamic toolbar Gecko can use the toolbar height to resolve vh units. (Note that the height without toolbar is used for ICB, resolving % units, etc.) With these we don't need to use setVirticalClipping and I suppose this issue is fixed. Note that I think we also need an API to tell whether the toolbar is static or dynamic from Gecko, or just adding the info into the toolbar height notification .

Yes, telling the main thread about the toolbar position and letting it position bottom-fixed items accordingly would be an alternative approach.

The current approach is to let the main thread position bottom-fixed items behind the toolbar, and have APZ fix up their positioning.

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

Yes, telling the main thread about the toolbar position and letting it position bottom-fixed items accordingly would be an alternative approach.

It's worth noting that, even if we do this, we'll still need the setVerticalClipping API. Otherwise, the position of bottom-fixed items would only be adjusted when we repaint, not on every composite (i.e. the movement wouldn't be smooth).

One of the things I realized as I was thinking about how to fix this, is that APZ actually has two codepaths where it applies transforms for hit-testing purposes (three if you count WebRender). I wrote some notes about this.

Updating the first codepath (ComputeTransformForNode()) to account for the fixed margins adjustment should be straightforward, at least for non-WebRender.

The second codepath (GetScreenToApzcTransform() + GetApzcToGeckoTransform()) is the trickier one. A general solution would involve updating the transform model to allow non-APZC nodes in the hit testing tree to have async transforms. This would be a fairly invasive change, and it would upset several assumptions:

1. The assumption that the transform that APZ applies to convert an input event form screen coordinates to Gecko coordinates is a function of the event's target APZC only. With the above change, it would be a function of the target node. (In particular, in the common case of a scrollable page with a bottom-fixed item, it would depend on whether we hit the fixed item's node.)
2. The assumption that transforms between APZCs ("ancestor transforms") only change during hit testing tree updates. We would no longer be able to cache the ancestor transforms the way we do today.

To avoid having to make these invasive changes, I'm thinking of a more targeted approach, that relies on the fact the transform representing the dynamic toolbar's movement is basically at the top level, and can be applied after the entire screen-to-gecko transform matrix. This way, we can apply it as a translation directly in ReceiveInputEvent, and avoid having to modify the screen-to-gecko transform itself.

Tracking for GV's October sprint

Some WIP patches for this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7263aad80329da65a4d34d811bd37753d9f530ce

It's still only used on Android, but keeping it and all code that interacts
with it inside an #ifdef is too much trouble for a tiny amount of space
saving.

Depends on D48369

Depends on D48370

Depends on D48372

Depends on D48373

Depends on D48374

Depends on D48375

Depends on D48376

Depends on D48377

Depends on D48378

This is a relic from earlier times when GetAPZCAtPoint() was recursive.

Depends on D48379

Depends on D48380

Depends on D48381

Depends on D48382

Populating them is hooked up for non-WebRender, with a comment outlining
possible implementation strategies for WebRender.

Depends on D48383

Only touch events are handled at this time.

Depends on D48384

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

Fenix's approach to the dynamic toolbar, however, involves an async transform that's persistent as long as the toolbar is partially or fully on-screen. This means we have to obey it during hit testing, which will require some extra machinery. It will also require support in WebRender, like the rendering part.

Filed bug 1586843 for the WebRender part.

Though I might be still misunderstanding what the problem here is, this problem might be fixed by making VisualViewport behavior match Chrome's. On Chrome, VisualViewport height is dynamically changed by the dynamic toolbar transition (and VisualViewport.onresize is repeatedly called during the transition). So to me, once after we made it happen we no longer need setVerticalClipping API?

(Note that on current GeckoView with CoordinatorLayout VisualViewport height is including the toolbar height regardless of whether it's dynamic or static)

https://hg.mozilla.org/mozilla-central/rev/7bbff3ca9c53
https://hg.mozilla.org/mozilla-central/rev/f8a125f2969e
https://hg.mozilla.org/mozilla-central/rev/daefc4088b5b
https://hg.mozilla.org/mozilla-central/rev/6310462ffafb
https://hg.mozilla.org/mozilla-central/rev/0674fa5af30f
https://hg.mozilla.org/mozilla-central/rev/a8a4f618d3d3
https://hg.mozilla.org/mozilla-central/rev/67c4bc35fc74
https://hg.mozilla.org/mozilla-central/rev/00824be6572b
https://hg.mozilla.org/mozilla-central/rev/597c42949166
https://hg.mozilla.org/mozilla-central/rev/7c1b57fdf9e2
https://hg.mozilla.org/mozilla-central/rev/ff8e1fa23dea
https://hg.mozilla.org/mozilla-central/rev/50fb72cc4a73
https://hg.mozilla.org/mozilla-central/rev/5482a581ab19
https://hg.mozilla.org/mozilla-central/rev/5b65905fba5c
https://hg.mozilla.org/mozilla-central/rev/824e732b442a
https://hg.mozilla.org/mozilla-central/rev/3c3c35952f7b
https://hg.mozilla.org/mozilla-central/rev/6506806b3bf8

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

Though I might be still misunderstanding what the problem here is, this problem might be fixed by making VisualViewport behavior match Chrome's. On Chrome, VisualViewport height is dynamically changed by the dynamic toolbar transition (and VisualViewport.onresize is repeatedly called during the transition). So to me, once after we made it happen we no longer need setVerticalClipping API?

I'm not sure what the relevance of VisualViewport is here. Fixed-position elements are attached to the layout viewport, not the visual viewport.

Right, but the problem doesn't happen in the case where the position:fixed elements are outside of the visual viewport, right? A problem I can see is that (at least from the perspective of a web compat issue) with the current dynamic toolbar implementation visual viewport height is including the toolbar height (I am saying scale=1 case). Oh, hmm. You are right in such case the layout viewport is also including the toolbar height, bug 1586144 should fix the problem, I think (I was assuming bug 1586986 should fix it).

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

in such case the layout viewport is also including the toolbar height, bug 1586144 should fix the problem, I think

What will be the layout viewport height after bug 1586144:

• Always including the toolbar height?
• Always excluding the toolbar height?
• Changes depending on whether the toolbar is hidden or shown?

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

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

in such case the layout viewport is also including the toolbar height, bug 1586144 should fix the problem, I think

What will be the layout viewport height after bug 1586144:

• Changes depending on whether the toolbar is hidden or shown?

Chrome behavior looks this. At least for the case of position:fixed; bottom: 0 elements. But it's hard to tell without debugging Chrome whether the layout viewport height is static or dynamic. What I am currently supposing is that the layout viewport height is static just like ICB, but position:fixed elements stick to the dynamic toolbar transitions.

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

What I am currently supposing is that the layout viewport height is static just like ICB,

Well, the layout viewport height is static as if the toolbar is there, i.e. excluding the toolbar height.

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

What I am currently supposing is that the layout viewport height is static just like ICB, but position:fixed elements stick to the dynamic toolbar transitions.

That would be my guess too.

Our current mechanism for making position:fixed elements stick to the bottom during dynamic toolbar transitions is the setVerticalClipping API.

Even if we add a mechanism for changing the position of fixed elements in the main thread, that will only be updated during a paint, and we'll need the setVerticalClipping API to cover the cases where we get a composite (that changes the dynamic toolbar position relative to the last paint) without a paint.

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

Well, the layout viewport height is static as if the toolbar is there, i.e. excluding the toolbar height.

If that's what Chrome does, that's interesting. I thought they had a constraint where the layout viewport had to be at least as large as the visual viewport.

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

Even if we add a mechanism for changing the position of fixed elements in the main thread, that will only be updated during a paint, and we'll need the setVerticalClipping API to cover the cases where we get a composite (that changes the dynamic toolbar position relative to the last paint) without a paint.

That's what I am concerned right now to fix bug 1586986. To fix bug 1586986, we also need to update visual viewport size in response to setVerticalClipping calls on the main thread, which results a reflow for position:fixed elements on the main thread. We probably need a machinery to bypass it.

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

That's what I am concerned right now to fix bug 1586986. To fix bug 1586986, we also need to update visual viewport size in response to setVerticalClipping calls on the main thread, which results a reflow for position:fixed elements on the main thread. We probably need a machinery to bypass it.

Ok, I understand now. We probably want to make sure setVerticalClipping does not affect the layout viewport (and that the placement of position:fixed elements is only affected by the layout viewport).

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

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

Well, the layout viewport height is static as if the toolbar is there, i.e. excluding the toolbar height.

If that's what Chrome does, that's interesting. I thought they had a constraint where the layout viewport had to be at least as large as the visual viewport.

Indeed, I may be wrong here. If Chrome's layout view height matches window.innerHeight (I am also saying scale=1 case),

• the layout viewport height is excluding the toolbar height in the case where the toolbar is completely shown
• the layout viewport height is excluding the toolbar height as if the toolbar is completely shown in the case of the toolbar transition
• the layout viewport height is whole visible area's height in the case where the toolbar is hidden

At least for window.innerHeight this is what Chrome does, so it looks the visual viewport height exeeds the layout viewport height during the toolbar transition, but I haven't actually checked.