From bug 1516048#c22, the banner for cookies/privacy banner for the theguardian.com homepage doesn't follow along with the dynamic toolbar when scrolling.

This might be the same case for reddit.com as well. See attached screenshot for both cases.

The banner on theguadian.com is a position:sticky element and it's correctly positioned with patches for bug 1586144, but it doesn't stick to the bottom when scrolling down the content. So maybe we are referring a wrong scroll layer there?

This bug is a blocker for re-enabling Fenix's dynamic nav bar.

The patches in bug 1552608 only handle position:fixed elements. position:sticky will need some additional work.

Outline of the work needed to support position:sticky:

• Propagate the layer attributes related to position:sticky (GetIsStickyPosition(), GetStickyScrollContainerId(), GetStickyScrollRangeOuter(), GetStickyScrollRangeInner()) to LayerMetricsWrapper and WebRenderScrollDataWrapper. (The WebRenderScrollDataWrapper implementations may or may not need to be filled in, depending on the approach taken in bug 1583380.)
• Propagate the sticky attributes further to HitTestingTreeNode.
• Revise APZCTreeManager::IsFixedToRootContent() to be IsFixedOrStuckToRootContent() which checks "is the element fixed to the root OR is the element sticky and currently stuck to the root". The "is currently stuck" part is somewhat tricky; it will involve querying the sticky scroll container ID, finding the corresponding APZC, and comparing its scroll offset to the inner and outer sticky ranges.

It looks like GetFixedPosSides() is already populated for sticky elements, so nothing further needs to be done there.

With the instruction what Botond provided in comment 4, I think I can fix this.

We'd still like to ship this with Fenix - is it still possible to get this into the 73 beta? We're trying to feature freeze soon so we can focus more on stability, so if not, we should talk about what these timelines look like.

Hiro, let me know if you need any help with this, or if you'd like me to take the bug.

This is corresponding the first two dots in comment 4.

This is probably corresponding to the last dot in comment 4, but it's very incomplete.

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

• Revise APZCTreeManager::IsFixedToRootContent() to be IsFixedOrStuckToRootContent() which checks "is the element fixed to the root OR is the element sticky and currently stuck to the root". The "is currently stuck" part is somewhat tricky; it will involve querying the sticky scroll container ID, finding the corresponding APZC, and comparing its scroll offset to the inner and outer sticky ranges.

What I couldn't finish up is this part, from the patch I posted in comment 9.

• gfxFloat diff =
• (apz::IntervalOverlap(translation.y, outer.Y(), outer.YMost()) -
• apz::IntervalOverlap(translation.y, inner.Y(), inner.YMost())) -

•   translation.y;
  

• if (diff == 0) {
• return Nothing();
• }
• return diff > 0.f ? Some(SideBits::eBottom) : Some(SideBits::eTop);

I am not sure this is correct or not, I haven't tested on hit testing.

Another thing I couldn't figure out a proper way is to obtain the proper SideBits value from the scrolled position in AsyncCompositionManager::AlignFixedAndStickyLayers(). In my understanding if the sticky node is stuck to the top of the scrollable layer it should be eTop, if the sticky node is stuck to the bottom of the layer, it should be eBottom. I have dumped sticky{Inner,Outer} values there and tried to understand how we can get the SideBits info with the inner/outer values, but I haven't yet figured out.

If both patches look close to finish, I will be happy to finish it, but if it's far from the finish, I'll be taken over happily.

(I wrote the patches and invested it before the last year's offs, so I might be mis-remebering what I did, and unfortunately local building reference-browser on today's m-c causes startup crash so I can't make sure right now whether both patches work properly on the current m-c)

Could you upload the patches to Phabricator please?

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

Another thing I couldn't figure out a proper way is to obtain the proper SideBits value from the scrolled position in AsyncCompositionManager::AlignFixedAndStickyLayers(). In my understanding if the sticky node is stuck to the top of the scrollable layer it should be eTop, if the sticky node is stuck to the bottom of the layer, it should be eBottom. I have dumped sticky{Inner,Outer} values there and tried to understand how we can get the SideBits info with the inner/outer values, but I haven't yet figured out.

There are two parts to the condition we need to check:

(is sticky relative to the bottom) && (is currently stuck)

The first part of the condition needs the SideBits. The SideBits cannot be inferred from the sticky scroll ranges, they need to be provided separately. They are stored on the layer in Layer::GetFixedPositionSides() (even for sticky layers; I realize this is probably not the best name) and propagated to the LayerMetricsWrapper and HitTestingTreeNode. (For WebRender, populating them in the WebRenderScrollDataWrapper needs to be hooked up for sticky elements, similar to this patch. We can handle that in another bug.)

The second part of the condition can be computed based on the current root scroll position and the sticky scroll ranges.

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

The SideBits cannot be inferred from the sticky scroll ranges,

(I mean, it may be possible to, but my point is, we don't need to, as we have the SideBits available.)

Depends on D60065

Chenxia: Will we need to uplift this to beta 73 to make it available for you to ship in Fenix?

If it's not too much trouble, then yes, I'd like to get it into 73 Beta so that we can get more testing on it, earlier, and have time to file and get things fixed.

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

The patches in bug 1552608 only handle position:fixed elements. position:sticky will need some additional work.

This diagnosis is incomplete: bug 1552608 only concerned hit testing, but position:sticky elements have a rendering problem as well.

The rendering part is due to bug 1546139 only handling position:fixed elements.

(Hiro's patches will fix both. I'm just documenting this because I was confused about it until now.)

Fenix might be taking another late beta around 1/29, but if it's not going to make it in because of all hands week, we'll just pick it up in 74. We will take an early 74 beta when it comes out in feb, for our 2/14 beta (and major Fennec Beta Migration).

However, if it's still possible to get it in for a pre-Beta-Migration release (e.g. 73 beta) we'll be able to test it in the wild before it hits our comparatively large Fennec Beta population.

https://hg.mozilla.org/mozilla-central/rev/a1034909acc8
https://hg.mozilla.org/mozilla-central/rev/24e033984b20

(In reply to Chenxia Liu [:liuche] from comment #19)

However, if it's still possible to get it in for a pre-Beta-Migration release (e.g. 73 beta) we'll be able to test it in the wild before it hits our comparatively large Fennec Beta population.

I think the uplift risk for these patches is acceptable, as they should only have an effect in configurations where the dynamic toolbar is enabled.

Hiro, if you agree, could you request uplift?

Chenxia, are we still in time to make the late beta you mentioned?

Indeed, those patches affect only on the environment which enables the dynamic toolbar, that means it's Android. So I agree, the uplift risk should be pretty low.

A cumbersome thing is that the patches here don't have automated tests so that it requires a manual test, but as you guys know Fenix doesn't have the bottom dynamic toolbar yet, so we have no way to do the manual test on nightly builds. I can (hopefully) provide a modified Fenix APK, but does it mean a reasonable manual test? I am not sure.

Julien, can we accept the code which is basically only for Android as a beta uplift without tests?

We're in RC week for Fx73. Let's let this ride with 74.

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