1630274 - Fix dynamic-toolbar-sticky-position-6b.html on non-WR Android

Status: RESOLVED INVALID

(Core :: Panning and Zooming, defect, P3)

Description

[Kartikaya Gupta (email:[e-mail])]

I have patches for bug 1627362 and bug 1627326 that result in the Android + non-WR codepath passing most of the tests that were added in bug 1628484. However there remain two failures: dynamic-toolbar-sticky-position-6b.html with a simulated "top" dynamic toolbar, and the mirror image test dynamic-toolbar-sticky-position-1c.html with a simulated "bottom" dynamic toolbar.

I investigated the failures and I believe I understand why it's happening, but the fix is nontrivial so I'm spinning this out into a separate bug. (Also it only happens on the non-WR codepath so I care about it less.)

The problem is basically that when AsyncCompositionManager adjusts the position of sticky/fixed items, it does so in two "passes". First it does the call here which finds any rootmost fixed/sticky descendants of the current async-transformed layer, and "unapplies" the async transform to those descendants so that they remain visually fixed. And then, it does the call here which is intended to further adjust the sticky/fixed items based on the dynamic toolbar margins.

The problem is that the second call uses a translation value in order to figure out the sticky adjustment, but that translation doesn't include the translation applied to the layer from the first pass. Without including that translation, the IntervalOverlap call produces an incorrect sticky adjustment, and so the sticky item isn't shifted by the right amount.

I think the most correct solution here would be to unify the two passes into a single pass. If done properly, this will result in code simplification and more correct behaviour. However doing this requires reorganizing the code pretty heavily which is a bit scary. An alternate hacky approach would be to try and figure out the already-applied translation when we're in the second pass, and include that into the computed translation value. But I'm not sure if e.g. async zoom or animation transforms higher up in the layer tree will affect the translation value, so it might be nontrivial to deduce the already-applied translation.

Also as an aside, it seems like in some scenarios the fixedLayerMargins used in the first pass is always empty, because it only gets set inside the block where we "find the root" and it's not at all clear to me any more what circumstances that happens under. It could be that with Fennec gone a lot of this code can simply be deleted, and that might simplify the problem greatly.

Comment 1

[Dan Robertson (:dlrobertson)]

In a WR-only world, this issue is no longer valid.