Closed Bug 1494801 Opened 3 years ago Closed 3 years ago
Scrolled content doesn't repaint in cs
.chromium .org (with layout .css .contain .enabled=true, for 'contain:paint')
STR: * Go to https://cs.chromium.org/chromium/src/third_party/polymer/v1_0/components-chromium/paper-fab/paper-fab.html * Try to scroll down. Expected: * Content is immediately available. Actual result: * Content doesn't get repainted until you force it by, e.g., clicking on the page. I suspect this is a recent regression.
This is a paint containment issue. The scroller has contain: paint. Disabling layout.css.contain.enabled fixes it.
Summary: Scrolled content doesn't repaint in cs.chromium.org. → Scrolled content doesn't repaint in cs.chromium.org (with layout.css.contain.enabled=true, for 'contain:paint')
I'll try to look into this sometime in the next ~week, if nobody gets to it first. (Note that our 'contain:paint' impl is pretty trivial -- it's just an early-return in nsFrame.h "ShouldApplyOverflowClipping()" and in nsFrame.cpp "IsStackingContext()".)
Looks like ShouldApplyOverflowClipping is just wrong for scrollable frames, see bug 1178382. Markus explained the reason for that in the patch for that bug, probably it's still up-to-date.
I think we don't need to bother returning true from ShouldApplyOverflowClipping() here, actually! (Or for any "contain:paint; overflow: [non-visible]" frames). I'm pretty sure we don't need the scrollport to do any "bonus" contain:paint clipping (via ShouldApplyOverflowClipping), because: - the scrollport will already clip any overflowing content (and turn it into scrollable overflow) - contain:paint makes us a fixed-pos containing block, so elements can't escape that scrollport by reparenting themselves to a further-up-the-tree containing block. There may also be some general stuff to fix for scrollable frames with ShouldApplyOverflowClipping()==true, as hinted at in comment 3. But I suspect the simplest solution here is just to make scrollable frames "opt out" of the IsContainPaint() logic in that function, since it's unnecessary.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
The contain:paint clipping would be redundant and hence unnecessary in this scenario, because: - Scroll frames already clip their descendant frames. - contain:paint has other (non-clipping-related) effects that prevent descendant frames from escaping the scrollframe ancestor. So, no further clipping is required. This is a behavior change - it works around an issue that makes us fail to repaint mousewheel-scrolled content inside of any scrollframe that returns true from ShouldApplyOverflowClipping().
Green try run (though I've got a typo'd bug number in the commit message, which I've since fixed on phabricator): https://treeherder.mozilla.org/#/jobs?repo=try&revision=10dab11b49328c5d0190e419793054874c59ca28
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/5884c1ce6696 Don't add additional "contain:paint" clipping on scroll frames. r=mattwoodrow
You need to log in before you can comment on or make changes to this bug.