Closed Bug 1303761 Opened 3 years ago Closed 3 years ago

[APZ] Firefox doesn't repaint uncovered regions after scrollwheel scrolling, at BBC article with scrolling-images-and-text effects

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla52
Tracking Status
e10s ? ---
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 + verified
firefox52 --- verified

People

(Reporter: dholbert, Assigned: mstange)

References

Details

(Keywords: regression)

Attachments

(3 files)

Filing a bug for a recent Linux nightly scrolling regression.

STR:
 0. Ensure you have e10s enabled.

 1. Visit http://www.bbc.co.uk/news/resources/idt-6d083913-0bfb-4988-8cd8-d126fa6dcff1

 2. Scroll to the paragraph that begins with the phrase "As the internet". (Use Find-in-page, ctrl+F, to jump straight there if you like)

 3. Scroll up and down in the graphics just above that paragraph, paying special attention to the transparent bar that says "Headlines and reports from The People, The Liverpool Echo and The Guardian"

EXPECTED RESULTS:
That transparent bar should consistently be the bottom of the graphical image section.

ACTUAL RESULTS:
* When I scroll down the page, the transparent bar moves up, but it leaves behind some images after it which should have been cleared.  (If I click outside of the Firefox window, this section repaints as white)
* When I scroll back up the page, that transparent bar moves down, but it leaves behind some unpainted solid-black area. (If I click outside of the Firefox window, this section repaints as images.)


Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4b7cd5b39cca85efc4b8350520303605cb59efbe&tochange=b1dbce81bf3b6124577fb46414f811ec5f45f4e0

--> regression from bug 1012752

(Note: I ran into this bug when trying to reproduce bug 1302298. I'm not 100% sure it's the same thing that jyc was seeing there, and kats opted to focus that bug on an Android-specific issue; hence, I spun off this bug here for this recent regression that I'm seeing on Desktop.)
Flags: needinfo?(mstange)
Attachment #8792524 - Attachment description: screencast of bug → screencast of bug (there should never be any solid-black regions, or any images after "Headlines and reports" text-bar)
Attachment #8792524 - Attachment description: screencast of bug (there should never be any solid-black regions, or any images after "Headlines and reports" text-bar) → screencast of bug (there should never be any solid-black regions, or any image-background-area after "Headlines and reports" text-bar)
Requesting tracking for Firefox 51, since this is a regression in that release.
Keywords: regression
NOTE: For me (in a fresh profile), this bug only reproduces with *mouse scrollwheel* scrolling -- not with keyboard scrolling (uparrow/downarrow keys).
(The bug also goes away if I set the pref "layers.async-pan-zoom.enabled" = false, or if I disable e10s [which IIRC also implicitly disables async pan & zoom]).
Summary: Firefox leaves behind unpainted regions after scrolling, at BBC article with scrolling-images-and-text effects → [APZ] Firefox doesn't repaint uncovered regions after scrollwheel scrolling, at BBC article with scrolling-images-and-text effects
Attached file testcase
Looks like the workaround from bug 1284586 was not effective if there is an additional overflow:hidden involved. This testcase is basically attachment 8673015 [details], just with an additional overflow:hidden on the element with the clip.

I'll extend the workaround to cover this case, so that we can uplift the fix.

The real fix is in bug 1298218.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Flags: needinfo?(mstange)
I don't know why this problem only started to appear after bug 1012752. We should have had it all along.
Comment on attachment 8793118 [details]
Bug 1303761 - Disable paint skipping for all scroll frames between the fixed element and the scrolled clip.

https://reviewboard.mozilla.org/r/79896/#review78690
Attachment #8793118 - Flags: review?(tnikkel) → review+
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/60ee6e26982a
Disable paint skipping for all scroll frames between the fixed element and the scrolled clip. r=tnikkel
https://hg.mozilla.org/mozilla-central/rev/60ee6e26982a
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Hi :mstange,
Since this bug is a regression and also affects 51, do you consider to uplift this for 51 if this patch is not too risky?
Flags: needinfo?(mstange)
Track for 51+ as this is a regression.
Verified that the issue described in comment 0 no longer reproduces on latest Nightly 52.0a1 2016-09-22 under Ubuntu 14.04 64-bit, Mac OS X 10.11 and Win 10 64-bit. 

A short lag is still visible while scrolling.
Comment on attachment 8793118 [details]
Bug 1303761 - Disable paint skipping for all scroll frames between the fixed element and the scrolled clip.

Yes, this fix is definitely upliftable. I'm also requesting uplift to Beta; while we haven't seen pages in the wild hit this bug on Beta (which does not have the fix for bug 1012752), the incorrect code is in there and I wouldn't rule out that other pages could hit it independently of bug 1012752.

Approval Request Comment
[Feature/regressing bug #]: APZ, and somehow aggravated by bug 1012752
[User impact if declined]: incorrect painting during scrolling on some pages
[Describe test coverage new/current, TreeHerder]: okay-ish but this particular bug is not easily testable. bug 1298218 will fix it completely and add tests
[Risks and why]: low, just disabling an optimization ("paint skipping") for an unhandled case
[String/UUID change made/needed]: none
Flags: needinfo?(mstange)
Attachment #8793118 - Flags: approval-mozilla-beta?
Attachment #8793118 - Flags: approval-mozilla-aurora?
Hi Markus, according to the Fx50 status, this one is marked unaffected. Do we still need to uplift to Beta50?
Flags: needinfo?(mstange)
Comment on attachment 8793118 [details]
Bug 1303761 - Disable paint skipping for all scroll frames between the fixed element and the scrolled clip.

Ok, it seems this really does not affect Beta. From reading the code it seems like Beta *should* be affected, but I can't make the bug happen in it, neither on the BBC site nor on my testcase. So let's not uplift this to Beta.
Flags: needinfo?(mstange)
Attachment #8793118 - Flags: approval-mozilla-beta?
Comment on attachment 8793118 [details]
Bug 1303761 - Disable paint skipping for all scroll frames between the fixed element and the scrolled clip.

This patch fixes a regression. Take it in 51 aurora. And this also needs QE to verify.
Attachment #8793118 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
Verified as fixed using latest Developer Edition 51.0a2 under Win 10 64-bit, Mac OS X 10.11.
Marking as verified considering this and comment 14.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
See Also: → 1382534
You need to log in before you can comment on or make changes to this bug.