1338183 - display: contents should compute to inline for replaced elements and form controls

Status: NEW

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

This is a somewhat annoying bug to fix, but the CSSWG resolved this[1].

I guess we could make the display: contents style uncacheable, modify the display in place in the frame constructor's GetDataByTag, and rely in the fact that changing display reframes, or add a bit to the style context to note that, and ensure it's propagated when the element is restyled. Alternatively we could do another display-contents-only style adjustment in nsRuleNode, but that seems also not great.

Probably mark all the elements that are replaced content with a bit and adjust style properly would be the easiest solution (with proper assertions), and as a side effect will also allow us to skip all the DataByTag stuff completely when the element is not replaced content.

I don't intend to take this too soon, so if someone wants to work on it feel free to do so.

[1]: https://github.com/w3c/csswg-drafts/issues/540

Comment 1

[Boris Zbarsky [:bzbarsky]]

I really think the WG goofed here.  Not only is there the issue with caching/sharing, but also https://github.com/w3c/csswg-drafts/issues/1024 is a problem.

I personally think we should push back on this decision...

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I think we're on time to push back FWIW. I think it also seems reasonable to just compute as contents but act as an inline for replaced elements, which is what Gecko does now. In Blink I recently implemented it so it computed as inline, but that's inconsistent with pretty much any other display type (eg, |svg { display: grid }| acts as an inline).

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #2)
> I think we're on time to push back FWIW. I think it also seems reasonable to
> just compute as contents but act as an inline for replaced elements, which
> is what Gecko does now. In Blink I recently implemented it so it computed as
> inline, but that's inconsistent with pretty much any other display type (eg,
> |svg { display: grid }| acts as an inline).

Hmm, I'm wrong on this though. Anyway, any other display type on replaced content like svg computes to the specified value, not to another display type.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #3)
> Hmm, I'm wrong on this though. Anyway, any other display type on replaced
> content like svg computes to the specified value, not to another display
> type.

(And in gecko display: contents acts as an inline because of [1] if I'm not wrong)

[1]: http://searchfox.org/mozilla-central/source/layout/style/nsStyleStruct.h#2951

Comment 5

[Mats Palmgren (inactive)]

I fully agree with bz in comment 1.
I've commented on the original csswg issue which made this change:
https://github.com/w3c/csswg-drafts/issues/540#issuecomment-279055365

Comment 6

[Mats Palmgren (inactive)]

This is from the "[CSSWG] Minutes Seattle F2F 2017-01-13 Part III" just posted on www-style:
"
  RESOLVED: display:contents behaves as display:inline for all
            replaced elements and all form controls
"
https://lists.w3.org/Archives/Public/www-style/2017Feb/0062.html

So I believe the computed value should stay as is and our behavior is correct.
It seems to me this spec change doesn't correspond at all to what was actually resolved.

It's still unclear what "form controls" means though.

Comment 7

[Ilya]

It seems that the WG has clarified how `display:contents` should work for unusual elements, including form controls: https://drafts.csswg.org/css-display/#unbox. In particular, the spec now states that for `button` and `fieldset` elements `display:contents` should result in removing their own box with all its specific non-CSS UI decorations, and rendering their contents as normal (while preserving activation behavior, DOM events etc.). Current implementation in Chrome (behind the flag) already does this (example: https://jsfiddle.net/39h74fdj/5/). It also preserves the functionality of these elements (activation behavior etc.), which seems absolutely correct since the change of `display` affects only box tree. There are signals that web developers find this behavior extremely useful because it allows them to control the appearance of controls completely with CSS (e.g. https://github.com/whatwg/html/issues/2805).

Probably this bug should be edited according to the new state of the spec and Firefox should implement the behavior that Chrome did?

Comment 8

[Mats Palmgren (inactive)]

The new spec looks fine to me.  Specifically, it says that display:contents
"behaves as ...", rather than "computes to ...".

So I think this bug should be closed as invalid.  Instead, we should file
new bugs for cases we don't implement correctly per the new spec.
On bug per issue please -- layout bugs in "Layout: Form Controls",
and event / activation bugs in "DOM: HTML".

Comment 9

[Mats Palmgren (inactive)]

... and bugs concerning SVG in "SVG".
https://drafts.csswg.org/css-display/#unbox-svg