Closed Bug 1463593 (css-contain-layout) Opened 6 years ago Closed 5 years ago

[meta] Implement CSS "contain: layout"

Categories

(Core :: Layout, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Unassigned)

References

Details

(Keywords: dev-doc-complete, meta, Whiteboard: [layout:p1])

See https://drafts.csswg.org/css-contain/#containment-layout
This bug is on making our layout/reflow code react appropriately to "contain: layout".

(See also bug 1178895 which has a work-in-progress patch stack for an earlier draft of this spec.)
Status: NEW → ASSIGNED
Priority: -- → P3
Visit

http://www.gtalbot.org/BrowserBugsSection/CSS3Contain/

for 'contain: layout' tests. 

More tests with associated reference files will be developped.
This may want to end up as a metabug, with most or all of the work done in dependencies, since there are 5 kinda-independent requriements from the spec. (Each of which might be worth fixing as a standalone helper-bug, perhaps, ideally with its own targeted testcase(s) for that piece [unless we've pulled in web-platform-tests from upstream that already cover that piece].)

Here's an overview of what the spec's requirements are right now:

> If the element does not generate a principal box (as is the case with display: contents or display: none), or if the element is an internal table element other than display: table-cell, or if the element is an internal ruby element, or if the element’s principal box is a non-atomic inline-level box, layout containment has no effect. 

We need to be sure to exclude all of these types of boxes from getting any special treatment (as we're doing for paint & size containment).

> Otherwise, giving an element layout containment has the following effects:
>
> 1. The element establishes an independent formatting context.

(Note that this is a requirement of both contain:layout & contain:paint, so we should perhaps share code to achieve this.)

fantasai says that this means the element is a BFC (block formatting context), which probably means we want to set NS_BLOCK_FORMATTING_CONTEXT_STATE_BITS (search MXR to see how that is used & what it means).

For blocks, this has the following effects:
 - margin-collapsing should not happen across the boundary of the layout-contained box.
 - floats from other boxes should not influence layout inside the layout-contained box. (And vice versa, I think)

For grid, this needs to make the "subgrid" feature be ignored (subgrids should behave like they were normal standalone grids).  subgrid isn't fully implemented yet, see bug 1240834.

For other frames, this might not have any effect...

> 2. If a fragmentation context participates in layout containment [...]

I don't have a good feel for what this section means, and I filed https://github.com/w3c/csswg-drafts/issues/2840 for some clarification.

> 3. If the contents of the element overflow the element, they must be treated as ink overflow.

This would probably be implemented via adding some subtlety to nsFrame::ConsiderChildOverflow.  (Right now this method treats scrollable overflow on the child as scrollable overflow on the parent, and ink-overflow on the child as ink-overflow on the parent.  But on a box with layout-containment, we'll want to union *both* categories (from that box's kids) into our ink overflow.

> 4. The element acts as a containing block for absolutely positioned and fixed positioned descendants.

(Note that this is a requirement of both contain:layout & contain:paint, so we should perhaps share code to achieve this.)

I think for this, we'd just want to extend the IsContainPaint() check in nsStyleDisplay::HasFixedPosContainingBlockStyleInternal().

> 5. Forced breaks are allowed within elements with layout containment, but do not propagate to the parent as otherwise described in CSS Fragmentation Module Level 3 §break-between.

(Note: I think we support this sort of breaking via the older property-names "page-break-before" and "page-break-after" forced breaks right now, though they're called nsStyleDisplay::mBreakBefore/mBreakAfter internally in our C++ code.  We'd want to look at usages of those variables in layout to figure out what needs to change.)
Blocks: 1471758
Morgan & Yusuf: when you run out of work on the other containment bugs you're looking at (or if you get stuck waiting for a response on something and you're looking for something else to do), feel free to spin off pieces of this bug [comment 2] as new helper-bugs, marked as blocking this one. (Bullet points 1 and 4 seem particularly straightforward to start with, due to being analogous to existing [I think] contain:paint functionality.)
Thanks for breaking things down! Yes, 1 and 4 as well as the exceptions where layout should not take any affect seem similar to pieces we have for other containment types. I've been dived into few issues with contain:style, and I'll try to keep that separate from this to prevent creating any complications. But I'll definitely be following this bug and hopefully can take on some of those tasks.
(In reply to Daniel Holbert [:dholbert] from comment #2)
> > 2. If a fragmentation context participates in layout containment [...]
> 
> I don't have a good feel for what this section means, and I filed
> https://github.com/w3c/csswg-drafts/issues/2840 for some clarification.

Update: per the GitHub issue linked above, this part relates to regions and nth-fragment syntax, which we don't support. So we don't have anything we need to do for this requirement.
Depends on: 1472919
Depends on: 1476495
Depends on: 1481641
Depends on: 1481951
Firefox 63.0a1 buildID=20180808100114 fails the 1st sub-test of 

http://test.csswg.org/harness/test/css-contain-1_dev/single/contain-layout-008/

while it passes

http://test.csswg.org/harness/test/css-contain-1_dev/single/contain-paint-013/

Should I create a new bug report and mark it as blocking this bug?
(In reply to Gérard Talbot from comment #6)
> Should I create a new bug report and mark it as blocking this bug?

Lets wait until bug 1472919 lands, I think some of the formatting/stacking context stuff may get addressed there, too. 

Thank you for pointing this out!
FYI

Firefox 63.0a1 buildID=20180810220115 now PASSes the 1st sub-test of 

http://test.csswg.org/harness/test/css-contain-1_dev/single/contain-layout-008/
Depends on: 1483946
Blocks: 1487493
Depends on: 1490111
Depends on: 1491235
Depends on: 1495470
Whiteboard: [layout:p1]
Depends on: 1508441
Depends on: 1539586
Depends on: 1539589
Depends on: 1494100
No longer depends on: 1494100
Depends on: 1552287
Depends on: 1553548
No longer depends on: 1539586
Depends on: 1563050
Assignee: mreschenberg → nobody
Status: ASSIGNED → NEW

This feature shipped in Firefox 69 (with the implementation work happening in dependent bugs), so let's call this [meta] and close it out.

(yay!)

Summary: Implement CSS "contain: layout" → [meta] Implement CSS "contain: layout"
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.