1579336 - Page Layout issue in FF69 caused by implementing CSS 'contain: size' but not 'contain: style'

Status: RESOLVED DUPLICATE of bug 1463600

(Core :: Layout, defect, P3)

Description

[robertedgar]

Attachment: test1.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/76.0.3809.132 Safari/537.36

Steps to reproduce:

Create a div with two classes, the base class includes 'contain:size' and explicitly declares the height and the overriding class includes 'contain:style' and sets height to auto.

Actual results:

the div isn't displayed in FF69 because the height of the div is 0 but is displayed in FF68 and in any version Chrome.

In FF68 contain in both classes was invalid so it displayed.

In FF69 'contain:size' is valid but 'contain:style' is not therefore although the overriding class sets 'height:auto', 'contain:size' will still apply and cause the div to not render as its height is now dependent on its children.

Expected results:

The div should be displayed

Comment 1

[robertedgar]

Forgot to say, in the test case there are 3 divs colored red/green/red.
In FF68 and in Chrome all 3 divs show, in FF69 the first red div doesn't show.

Comment 2

[Boris Chiou [:boris]]

Looks like this is an issue on contain:size. I can reproduce this on Firefox 71.

Comment 3

[Boris Chiou [:boris]]

I think this is a regression. Add regressionwindow-wanted.

Comment 4

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4f6d8ed9e94809f911907725a708627e3faaed01&tochange=79e130a002553dc7a2e519c2bedcaf0d73d3e9c7

Comment 5

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

Here is the regression range:
Last Good: 2019-03-19
First Bad: 2019-03-20

Pushlog-url: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d15d511d7fed036d69d37cecbacdea779a983a34&tochange=708979f9c3f33c77fb2e9106054b215c77044a04

Comment 6

[Daniel Holbert [:dholbert]]

Just taking a look here, following up from bug 1463600 comment 58 (sorry for taking so long to take a look!):

As it turns out, the testcase here is effectively a roundabout way of demonstrating that contain:style is unsupported (and in particular, that it's rejected in the CSS parser). As a result, I'm duping this back to bug 1463600.

The element in question (the first div, <div class="classA classB">) has this from .classA:

  /* from .classA: */
  contain:strict;
  height:60px;
  /* from .classB: */
  height: auto;
  contain: paint style layout;

Here's how these properties are resolved:
(1) For 'contain': we don't support contain:style right now, so the element ends up with the earlier contain:strict declaration. We treat strict as size layout paint as required by https://drafts.csswg.org/css-contain-1/#valdef-contain-strict

(2) For 'height', we understand both declarations and use the last one per CSS cascading rules.

So the element ends up with contain:strict (which expands to include size) and height:auto; and that combination means the element correctly ends up as having 0 height.

To get the author's expected behavior here, we would need the parser to accept contain:style (or rather contain:paint style layout) so that we can accept that declaration and avoid falling back to the author's earlier contain:strict (which is the line that is causing the unwanted layout, when it gets properly-honored). Hence, this is essentially just bug 1463600.

(This superficially appears to be a regression since the testcase happens to also produce the author's expected behavior if contain is entirely ignored. So in that sense, it manifests as being a regression from the bug that enabled support for contain. But really, it's just that the testcase has a hard dependency on contain:size and contain:style -- the testcase happens to require that EITHER they're both supported or that neither is supported.)