Closed Bug 1803928 Opened 3 years ago Closed 3 years ago

container-type and content-visibility don't really trigger style containment

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
109 Branch
Tracking Flags:
Tracking Status
firefox109 --- fixed

People

(Reporter: Oriol, Assigned: Oriol)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Bug 1803928 - Fix style containment not triggered by 'contain'. r?emilio
48 bytes, text/x-phabricator-request
Details | Review
Bug 1803928 - Make mContain and mEffectiveContainment private. r?emilio
48 bytes, text/x-phabricator-request
Details | Review

container-type and content-visibility can trigger various kinds of containment, but this is done via nsStyleDisplay::EffectiveContainment, the computed value of contain is not affected.

This is a problem, because https://searchfox.org/mozilla-central/rev/49a56597a4ef27f2110b74a95443fa58ed74fd35/servo/components/style/style_adjuster.rs#282-288 adds the SELF_OR_ANCESTOR_HAS_CONTAIN_STYLE if the computed value of contain contains style:

        if box_style
            .clone_contain()
            .contains(SpecifiedValue::STYLE)
        {
            self.style
                .add_flags(ComputedValueFlags::SELF_OR_ANCESTOR_HAS_CONTAIN_STYLE);
        }

So container-type and content-visibility can't set this flag, which means that style containment doesn't really work:

<!DOCTYPE html>
<style>
main { counter-reset: c 0 }
main::after { content: counter(c) }
main > div::before { content: ""; counter-increment: c }
</style>
<main><div style="content-visibility: auto"></div></main>
<main><div style="container-type: size"></div></main>
<main><div style="contain: style"></div></main>

Expected:

0
0
0

Actual:

1
1
0

'container-type' and 'content-visibility' can trigger various kinds of
containment, but this was done via nsStyleDisplay::EffectiveContainment,
to avoid affecting the computed value of 'contain'.

This was fine except for style containment, which needs to set the flag
SELF_OR_ANCESTOR_HAS_CONTAIN_STYLE in order to really work, but this was
only done with 'contain: style'.

So this patch removes nsStyleDisplay::EffectiveContainment, and instead
uses two fields for containment: mContain and mEffectiveContainment.
This is somewhat analogous to mDisplay and mOriginalDisplay, though in
that case the computed value is the modified display.

Also fixes a typo in IsContainStyle() that made it return true for any
kind of containment, not just for style containment.

Backed out for causing content-visibility failures.

[task 2022-12-04T21:49:00.668Z] 21:49:00     INFO - TEST-PASS | /css/css-contain/content-visibility/content-visibility-018.html | Content Visibility: hit testing (composited with a composited child) 
[task 2022-12-04T21:49:00.668Z] 21:49:00     INFO - TEST-UNEXPECTED-FAIL | /css/css-contain/content-visibility/content-visibility-026.html | content-visibility:hidden does not affect computed value of 'contain' - assert_equals: expected "none" but got "size layout style paint"
[task 2022-12-04T21:49:00.668Z] 21:49:00     INFO - @http://web-platform.test:8000/css/css-contain/content-visibility/content-visibility-026.html:21:16
[task 2022-12-04T21:49:00.668Z] 21:49:00     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2591:25
[task 2022-12-04T21:49:00.668Z] 21:49:00     INFO - test@http://web-platform.test:8000/resources/testharness.js:628:30
[task 2022-12-04T21:49:00.668Z] 21:49:00     INFO - @http://web-platform.test:8000/css/css-contain/content-visibility/content-visibility-026.html:17:5
[task 2022-12-04T21:49:00.669Z] 21:49:00     INFO - 
[task 2022-12-04T21:49:00.669Z] 21:49:00     INFO - TEST-UNEXPECTED-FAIL | /css/css-contain/content-visibility/content-visibility-026.html | content-visibility:auto does not affect computed value of 'contain' - assert_equals: expected "none" but got "size layout style paint"
[task 2022-12-04T21:49:00.669Z] 21:49:00     INFO - @http://web-platform.test:8000/css/css-contain/content-visibility/content-visibility-026.html:25:16
[task 2022-12-04T21:49:00.669Z] 21:49:00     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2591:25
[task 2022-12-04T21:49:00.669Z] 21:49:00     INFO - test@http://web-platform.test:8000/resources/testharness.js:628:30
[task 2022-12-04T21:49:00.669Z] 21:49:00     INFO - @http://web-platform.test:8000/css/css-contain/content-visibility/content-visibility-026.html:24:5
[task 2022-12-04T21:49:00.748Z] 21:49:00     INFO - TEST-OK | /css/css-contain/content-visibility/content-visibility-026.html | took 807ms
Flags: needinfo?(oriol-bugzilla)

During the review I accidentally changed set_effective_containment into set_contain.

Flags: needinfo?(oriol-bugzilla)
Pushed by smolnar@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/de8645ec2cb5 Fix style containment not triggered by 'contain'. r=emilio https://hg.mozilla.org/mozilla-central/rev/590d11125c31 Make mContain and mEffectiveContainment private. r=emilio

https://hg.mozilla.org/mozilla-central/rev/de8645ec2cb5
https://hg.mozilla.org/mozilla-central/rev/590d11125c31

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
status-firefox109: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 109 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: