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

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

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.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screencast of bug (there should never be any solid-black regions, or any image-background-area after "Headlines and reports" text-bar)

Here's a screencast showing the bug.

Comment 2

[Daniel Holbert [:dholbert]]

Requesting tracking for Firefox 51, since this is a regression in that release.

Comment 3

[Daniel Holbert [:dholbert]]

NOTE: For me (in a fresh profile), this bug only reproduces with *mouse scrollwheel* scrolling -- not with keyboard scrolling (uparrow/downarrow keys).

Comment 4

[Daniel Holbert [:dholbert]]

(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]).

Comment 5

[Markus Stange [:mstange]]

Attachment: 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.

Comment 6

[Markus Stange [:mstange]]

I don't know why this problem only started to appear after bug 1012752. We should have had it all along.

Comment 7

[Markus Stange [:mstange]]

Attachment: Bug 1303761 - Disable paint skipping for all scroll frames between the fixed element and the scrolled clip.

Review commit: https://reviewboard.mozilla.org/r/79896/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79896/

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 8793118 [details]
Bug 1303761 - Disable paint skipping for all scroll frames between the fixed element and the scrolled clip.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79896/diff/1-2/

Comment 9

[Timothy Nikkel (:tnikkel)]

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/60ee6e26982a

Comment 12

[Gerry Chang [:gchang]]

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?

Comment 13

[Gerry Chang [:gchang]]

Track for 51+ as this is a regression.

Comment 14

[Petruta Horea [:phorea], Desktop QA]

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 15

[Markus Stange [:mstange]]

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

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Markus, according to the Fx50 status, this one is marked unaffected. Do we still need to uplift to Beta50?

Comment 17

[Markus Stange [: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.

Comment 18

[Gerry Chang [:gchang]]

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.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/81325da0086e

Comment 20

[Petruta Horea [:phorea], Desktop QA]

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.