While writing bug 1472919 comment 26 I noticed that this code: https://searchfox.org/mozilla-central/rev/196560b95f191b48ff7cba7c2ba9237bba6b5b6a/layout/style/ComputedStyle.cpp#228-252 is broken. In particular, it does the wrong thing when IsFixedPosContainingBlockForAppropriateFrame is true but IsFixedPosContainingBlock is actually false. For example, if an inline element has 'perspective' set (which doesn't apply) and has a dynamic change of 'filter' (which should change whether the element establishes a containing block), then this code will remove the UpdateContainingBlock hint even though the hint should not be removed, since the dynamic change of 'filter' does change whether it establishes a containing block (since 'perspective' doesn't apply). I think the fix is to separately test the parts that always apply and test the parts that don't apply to inlines. (So far I've only inspected code; haven't tested.)

We actually don't trigger this bug right now because of another bug: nsCSSFrameConstructor::ConstructInline doesn't actually call display->IsAbsPosContainingBlock(newFrame) (like, say, ConstructTable, ConstructDocElementFrame, etc.); it just checks for positioning.

Chrome behaves correctly (clicking "Toggle Filter" toggles the abs-pos containing block behavior), so I think we should just fix both bugs.

Turns out this was my fault to begin with, in bug 1251075.

It turns out making this change pass tests requires fixing the review comment from bug 1472919 that originally led me down this rabbit hole, since otherwise these patches make us fail: TEST-UNEXPECTED-FAIL | /css/css-contain/contain-paint-002.html | Testing http://web-platform.test:8000/css/css-contain/contain-paint-002.html == http://web-platform.test:8000/css/reference/pass_if_pass_below.html TEST-UNEXPECTED-FAIL | /css/css-contain/contain-paint-011.html | Testing http://web-platform.test:8000/css/css-contain/contain-paint-011.html == http://web-platform.test:8000/css/reference/ref-filled-green-100px-square.xht TEST-UNEXPECTED-FAIL | /css/css-contain/contain-paint-012.html | Testing http://web-platform.test:8000/css/css-contain/contain-paint-012.html == http://web-platform.test:8000/css/reference/ref-filled-green-100px-square.xht While thinking about fixing this, I filed a bunch of spec issues.

This fixes the regression of three web-platform-test reftests: testing/web-platform/tests/css/css-contain/contain-paint-002.html testing/web-platform/tests/css/css-contain/contain-paint-011.html testing/web-platform/tests/css/css-contain/contain-paint-012.html that was caused by patch 1, but it's written on top of the code in patches 2 and 3 so it's easier to fix afterwards.

The basic change here is making nsCSSFrameConstructor::ConstructInline use the function nsIFrame::IsAbsPosContainingBlock rather than testing for only one of the conditions in it (being relatively or absolutely positioned). The rest of the code changes follow from that change. I tested locally that the added test fails without the patch and passes with it (either with or without the next patch). Note that this causes a regression of three web-platform-test reftests: testing/web-platform/tests/css/css-contain/contain-paint-002.html testing/web-platform/tests/css/css-contain/contain-paint-011.html testing/web-platform/tests/css/css-contain/contain-paint-012.html which will be fixed in patch 4, since that fix is easier to write after patch 2.

This fixes a rather subtle bug. What the underlying code here is trying to do is remove nsChangeHint_UpdateContainingBlock when some properties that influence whether a frame is a containing block for absolutely positioned or fixed positioned elements have changed, but the final calculation of being a containing block has not changed. However, the old code was using a function that tested whether the style could *possibly* lead to a frame being a containing block. Some of the properties (like the transform properties) that lead to being a containing block, for example, don't apply to non-replaced inlines. Some, however, do (such as 'filter'). So if there's a dynamic change adding or removing a filter, on an inline that also has an *ignored* transform property (like 'transform' or 'perspective') set, then the code prior to this patch causes us to remove the UpdateContainingBlock hint. This patch fixes things by testing whether being a containing block could have changed for *any* type of frame, by separately testing the changes. The added tests fail without the patch and pass with the patch, viewed in isolation. However, without the previous patch, test 003 passes. Test 003 also fails in Chrome (but 001 and 002 pass).

This is needed for patch 4. This is based both on the wording in the spec and the discussion in https://github.com/w3c/csswg-drafts/issues/2987

Comment on attachment 8998059 [details] Bug 1479859 patch 1 - Make inline frames be abs-pos containing blocks for reasons other than being relatively positioned. r=emilio Emilio Cobos Álvarez (:emilio) has approved the revision. https://phabricator.services.mozilla.com/D2813

Comment on attachment 8998060 [details] Bug 1479859 patch 2 - Send nsChangeHint_UpdateContainingBlock when containing block-ness changes due to one property change, while another property that might trigger containing block-ness doesn't do so because of the frame type. r=emilio Emilio Cobos Álvarez (:emilio) has approved the revision. https://phabricator.services.mozilla.com/D2814

Comment on attachment 8998061 [details] Bug 1479859 patch 3 - Add an nsIFrame::IsFrameOfType bit to say whether frames support contain:layout and contain:paint. r=dholbert Daniel Holbert [:dholbert] has approved the revision. https://phabricator.services.mozilla.com/D2815

Comment on attachment 8998058 [details] Bug 1479859 patch 4 - Test becoming a containing block for contain:paint only for those frames that support it. r=dholbert Daniel Holbert [:dholbert] has approved the revision. https://phabricator.services.mozilla.com/D2812

So I think I've addressed all of the review comments except for Emilio's request for better function names on patch 2... I'm not sure I have any better ideas. (Yes, they're pretty verbose!)

https://hg.mozilla.org/integration/mozilla-inbound/rev/950df70cf532fa9ac658ffd3fa3d022eba9bc4c1 Bug 1479859 patch 1 - Make inline frames be abs-pos containing blocks for reasons other than being relatively positioned. r=emilio https://hg.mozilla.org/integration/mozilla-inbound/rev/55c49d4fe2874a2306dc36db6c86e287fcc7aa11 Bug 1479859 patch 2 - Send nsChangeHint_UpdateContainingBlock when containing block-ness changes due to one property change, while another property that might trigger containing block-ness doesn't do so because of the frame type. r=emilio https://hg.mozilla.org/integration/mozilla-inbound/rev/8e24328ba71484dd144b9d0d2fdd287a9327c1b4 Bug 1479859 patch 3 - Add an nsIFrame::IsFrameOfType bit to say whether frames support contain:layout and contain:paint. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/6897f3935cc77f531ce4b1ac5f5b6febdb010c58 Bug 1479859 patch 4 - Test becoming a containing block for contain:paint only for those frames that support it. r=dholbert

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12347 for changes under testing/web-platform/tests

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

It looks like the web-platform-tests test-verify jobs [Tier 2] failed on Mac and Windows because of antialiasing differences (subpixel vs. grayscale AA) between test and reference. However, it looks like the actual web-platform-tests reftests runs don't *run* the tests in the CSS directory at all... maybe because of these differences? So I *think* no Tier-1 jobs will fail, and I suspect the Tier-2 job fails only when the test gets touched.

(I've asked jgraham where the bug is for most web-platform-tests reftests not being run on Mac and Windows... and I'm curious if the reason is the same reason that caused these test-verify jobs to fail.)

https://hg.mozilla.org/mozilla-central/rev/950df70cf532 https://hg.mozilla.org/mozilla-central/rev/55c49d4fe287 https://hg.mozilla.org/mozilla-central/rev/8e24328ba714 https://hg.mozilla.org/mozilla-central/rev/6897f3935cc7

Upstream PR merged