858550 - Inverse scrolling on fixed position header

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Ian Barlow (:ibarlow)]

Somehow, scrolling down a page on Vimeo can cause the site header to creep down the screen. Example video: http://cl.ly/0o3N3H1T011H

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Dont transform descendents of fixed layers for async fixed-layer scrolling

Can't quite believe I did this in the first place. Hopefully a pretty no-brainer review, r?nrc in case the shadow layers refactor branch needs to mirror the change.

Comment 2

[Chris Lord [:cwiiis]]

Just tested, this same bug exists on FirefoxOS, but I think they use this same code now? (it used to be duplicated elsewhere in the tree, but I can't remember off-hand where that was)

Adding needinfo?benfrancis (if you don't know, please pass it on to someone that might :))

This is a really low-risk, high-impact fix, let's uplift it wherever we can.

Comment 3

[Ben Francis [:benfrancis]]

Anthony, do you know the answer to this question? I'm not sure who else to ask.

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Yes, b2g uses the same code. A group of us were just looking at this code last week and discussing that it looked like it needed a return statement.

Comment 5

[Chris Lord [:cwiiis]]

(In reply to Anthony Jones (:kentuckyfriedtakahe) from comment #4)
> Yes, b2g uses the same code. A group of us were just looking at this code
> last week and discussing that it looked like it needed a return statement.

Cool, so what's the process of getting this into b2g? I would say that it's both important and trivial enough to warrant an uplift.

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking for b2g18 - please get this landed & verified by qa on master, then nominate for uplift to v1.1 by using the approval-b2g18? flag

Comment 7

[Nick Cameron [:nrc]]

Comment on attachment 733846 [details] [diff] [review]
Dont transform descendents of fixed layers for async fixed-layer scrolling

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

Should be no different with the refactoring, thanks for putting it my way.

Comment 8

[Chris Lord [:cwiiis]]

Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/463440611ad6

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/463440611ad6

Comment 10

[Chris Lord [:cwiiis]]

Comment on attachment 733846 [details] [diff] [review]
Dont transform descendents of fixed layers for async fixed-layer scrolling

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Fixed-pos elements appear to 'run away' when scrolling
User impact if declined: Weird rendering while scrolling
Testing completed (on m-c, etc.): Tested locally, landed on m-c
Risk to taking this patch (and alternatives if risky): Pretty much zero risk
String or IDL/UUID changes made by this patch: None

Comment 11

[Chris Lord [:cwiiis]]

I have no idea about the b2g uplift process - given this is a one-line fix, do you think you could handle this please?

Comment 12

[bhavana bajaj [:bajaj]]

(In reply to Chris Lord [:cwiiis] from comment #10)
> Comment on attachment 733846 [details] [diff] [review]
> Dont transform descendents of fixed layers for async fixed-layer scrolling
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): Fixed-pos elements appear to 'run
> away' when scrolling
> User impact if declined: Weird rendering while scrolling
> Testing completed (on m-c, etc.): Tested locally, landed on m-c
> Risk to taking this patch (and alternatives if risky): Pretty much zero risk
> String or IDL/UUID changes made by this patch: None

Unclear if Fx21 is affected, can you please confirm ?

Comment 13

[Chris Lord [:cwiiis]]

Adding more affected flags - all current releases are affected.

Comment 14

[bhavana bajaj [:bajaj]]

(In reply to Chris Lord [:cwiiis] from comment #13)
> Adding more affected flags - all current releases are affected.

Thanks Chris !

Comment 15

[bhavana bajaj [:bajaj]]

Comment on attachment 733846 [details] [diff] [review]
Dont transform descendents of fixed layers for async fixed-layer scrolling

Approving the low risk patch on beta and adding qawanted,verifyme to get QA verification on beta/aurora for this.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/04ad45b6c07a
https://hg.mozilla.org/releases/mozilla-beta/rev/80a675915298

Comment 17

[Jason Smith [:jsmith]]

Tested on nightly on FxAndroid - scrolling down looks okay on vimeo, but I'm seeing the creeping happening on scroll up. Chris - Is that expected or is a followup needed?

No need for full branch verification - this is a one line change, so I'm pulling qawanted for a single branch verification to get it off the high priority requests that are showing up in our b2g queries.

Comment 18

[Chris Lord [:cwiiis]]

(In reply to Jason Smith [:jsmith] from comment #17)
> Tested on nightly on FxAndroid - scrolling down looks okay on vimeo, but I'm
> seeing the creeping happening on scroll up. Chris - Is that expected or is a
> followup needed?

This is probably something else, likely a bug related to the dynamic toolbar (probably bug 861246) - Does this happen in the most recent Nightly? (perhaps wait until tomorrow's, to catch the fix in bug 856932, assuming it sticks)

Comment 19

[Jason Smith [:jsmith]]

(In reply to Chris Lord [:cwiiis] from comment #18)
> (In reply to Jason Smith [:jsmith] from comment #17)
> > Tested on nightly on FxAndroid - scrolling down looks okay on vimeo, but I'm
> > seeing the creeping happening on scroll up. Chris - Is that expected or is a
> > followup needed?
> 
> This is probably something else, likely a bug related to the dynamic toolbar
> (probably bug 861246) - Does this happen in the most recent Nightly?
> (perhaps wait until tomorrow's, to catch the fix in bug 856932, assuming it
> sticks)

Yes, when I tested this, it was happening on today's nightly build (4/15).

I'll wait for the fix then with bug 856932.

Comment 20

[Chris Lord [:cwiiis]]

(In reply to Jason Smith [:jsmith] from comment #19)
> (In reply to Chris Lord [:cwiiis] from comment #18)
> > (In reply to Jason Smith [:jsmith] from comment #17)
> > > Tested on nightly on FxAndroid - scrolling down looks okay on vimeo, but I'm
> > > seeing the creeping happening on scroll up. Chris - Is that expected or is a
> > > followup needed?
> > 
> > This is probably something else, likely a bug related to the dynamic toolbar
> > (probably bug 861246) - Does this happen in the most recent Nightly?
> > (perhaps wait until tomorrow's, to catch the fix in bug 856932, assuming it
> > sticks)
> 
> Yes, when I tested this, it was happening on today's nightly build (4/15).
> 
> I'll wait for the fix then with bug 856932.

A video of what's happening would be helpful if that's something you can do without too much trouble - I'd test Nightly, but for whatever reason my Nightly is totally unusable and I don't want to clear data without debugging the problem.

Comment 21

[Johnny Stenback (:jst)]

I think this is bad enough that we should take this uplift. tef+

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/a0f3cb0dba1d
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/8a45cedcc417

Comment 23

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old