1153841 - Navigation bar on ESPN jumps around while panning

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Toolbar, defect, P1)

Description

[Brad Lassey [:blassey] (use needinfo?)]

If it's fixed position, we need to fix this. If not, we should reach out

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

Web compat issue here: https://github.com/webcompat/web-bugs/issues/836

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Mike do you have a bug for this other than the github issue? Do you want to close it/dup it?

Comment 3

[Mike Taylor [:miketaylr]]

We can dupe either this one or the other--I don't have a preference. By the sound of webcompat@836, their engineers are at least aware of our suggested workaround.

On one hand, position:sticky fixes the issue. On the other hand, Chrome Mobile performs wayyyy better than us with just position:fixed. Your call. :)

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

So this is not a web compat bug just because they can work around it.

Kevin, can you check quickly if this has regressed in the last 4 releases?

Comment 5

[Robert (rrayborn, he/him)]

From user feedback on ESPN:
* 37+    - we have reports that the menu bar is jittery.  It manifests worse on some pieces of hardware (cwang's N5 was far worse than my M8 or glind's cheap Samsung)
* 36     - we have a single report of "scrolling issues"
* pre-36 - we have reports that the menu is completely unresponsive and/or renders strangely.  Users are often not able to scroll at all.  Some users report that the page has a ton of excess scrollable space in the right margin.

This could either be correlated to dates or versions, I don't have enough data to make a strong claim.
Here's a link to some of the ESPN data on Input: https://input.mozilla.org/en-US/?q=espn+%2B+%28scroll+bar+menu%29&date_start=2014-01-01&product=Firefox%20for%20Android

Comment 6

[Kevin Brosnan [Ex-Mozilla]]

Regression in 30.0a1

23:29.97 LOG: MainThread Bisector INFO Narrowed nightly regression window from [2014-02-25, 2014-02-28] (3 days) to [2014-02-27, 2014-02-28] (1 days) (~0 steps left)
23:29.97 LOG: MainThread Bisector INFO Got as far as we can go bisecting nightlies...
23:29.97 LOG: MainThread Bisector INFO Last good revision: a98a1d78817f
23:29.97 LOG: MainThread Bisector INFO First bad revision: 58eca03214a6
23:29.97 LOG: MainThread Bisector INFO Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a98a1d78817f&tochange=58eca03214a6

Comment 7

[Kevin Brosnan [Ex-Mozilla]]

Tried bisecting via hg but I don't seem to have a working 30.0a1 build environment. If it is really needed I can spend some time on this.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

kats, is it useful to bisect this further?

Comment 9

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

Probably not. It's old enough that the code is going to be quite different now.

Comment 10

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

I took a quick look at this; there is a 3D transform on the fixed-pos item which is causing the problem (because our code in AsyncCompositionManager that deals with keeping fixed-pos items fixed doesn't deal with 3D transforms). The CSS for the page has a perspective:800px on the fixed-pos item, and if I remove that it works fine.

Comment 11

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

Attachment: Deal with 3D transforms for fixed-pos layers as well

I don't really see any reason to restrict the handling to 2D transforms, and dealing with 3D transforms as well fixes the issue on this page.

Comment 12

[Benoit Girard (:BenWa)]

Comment on attachment 8624838 [details] [diff] [review]
Deal with 3D transforms for fixed-pos layers as well

Review of attachment 8624838 [details] [diff] [review]:
-----------------------------------------------------------------

This looks like a good candidate to add a test. I can see us regressing this quite easily. Consider adding a test if-and-only-if it's pragmatic.

Patch looks good but I'm a bit surprised there are not any tricky cases to handle for 3D. This patch just removes the 2d checks essentially.

Comment 13

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

(In reply to Benoit Girard (:BenWa) from comment #12)
> This looks like a good candidate to add a test. I can see us regressing this
> quite easily. Consider adding a test if-and-only-if it's pragmatic.

So I thought it would be pretty straightforward to write a test for this. I copied layout/reftests/async-scrolling/position-fixed-1*.html to -3*.html and modified it so that the fixed-pos element had a 3D transform. On desktop however, the layout.scroll.root-frame-containers pref is set to false by default and on Fennec it's true (until bug 1158392 lands anyway), and that affects the structure of the layer tree that results. On desktop the fixed-pos layer ends up being a sibling of the async-scrolled layer whereas on Fennec it's a child, and that's relevant to the bug. This also implies that fixing bug 1158392 will fix this issue as well.

Also, the 3D transform that I induced caused assertion failures in layout (which is bad, I filed bug 1176355 for it) and I'm not sure how else to induce a 3D transform on layer. I tried copying the CSS properties from the ESPN page but it didn't work - I must be missing something (and I don't really understand the world of 3D CSS transforms enough to figure it out).

> Patch looks good but I'm a bit surprised there are not any tricky cases to
> handle for 3D. This patch just removes the 2d checks essentially.

I'm sure there are some tricky cases, but again I don't know enough about the world of 3D to really say. Hopefully mattwoodrow can chime in on that.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/07e102889997

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/07e102889997

Comment 16

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

Is this something we want to uplift?