web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
5.01k stars 3.11k forks source link

Are css/css-contain/contain-layout-baseline-005.html css/css-contain/contain-layout-button-001.html correct? #45889

Open bfgeek opened 6 months ago

bfgeek commented 6 months ago

I'm currently investigating making buttons block-layout by default in Blink.

As part of this work I'm fixing how we synthesize baselines for various form elements in an inline-context. (Note: this only applies to inline baselines, in all other contexts (flex/grid align-items: baseline) we synthesize based off the border-box edges).

Broadly speaking everything synthesizes off the margin-box edges except:

When containment is applied, we all seem to be synthesizing button like things off the margin-box edge.

I think this is incorrect per-spec:

https://drafts.csswg.org/css-contain-1/#containment-layout

For the purpose of the vertical-align property, or any other property whose effects need to relate the position of the layout containment box's baseline to something other than its descendants, the containment box is treated as having no baseline.

(To me) This reads as if:

baseline <button></button> <button style="contain:layout"></button>

Should be aligned the same way (e.g. the context-box edge(s)) as we are just treating them as having no baseline, and are instead synthesizing it.

@frivoal @dholbert @emilio Thoughts?

cc/ @mrego As you wrote (at least) one of the tests :P.

Ian

bfgeek commented 6 months ago

https://wpt.live/css/css-contain/contain-layout-baseline-005.html https://wpt.live/css/css-contain/contain-layout-button-001.html

bfgeek commented 6 months ago

https://chromium-review.googlesource.com/c/chromium/src/+/5479489 - if anyone is curious about the baseline patch.

dholbert commented 6 months ago

I'm not yet entirely sure what makes the most sense here, but one other situation that's worth comparing is what happens when these elements have an orthogonal writing-mode. In Firefox, that makes us synthesize a baseline using their border-edge, it seems.

Same WM as parent: https://www.software.hixie.ch/utilities/js/live-dom-viewer/saved/12648 Same WM as parent, plus contain:layout: https://www.software.hixie.ch/utilities/js/live-dom-viewer/saved/12649 (Firefox uses the bottom content-edge of in the first case, vs. bottom margin-edge in the second case, as IanK noted)

Orthogonal WM to parent: https://www.software.hixie.ch/utilities/js/live-dom-viewer/saved/12647 Orthogonal WM to parent, plus contain:layout: https://www.software.hixie.ch/utilities/js/live-dom-viewer/saved/12650 (Firefox renders these two^ identically, using the bottom border-edge to synthesize the baseline)

dholbert commented 6 months ago

Chrome (v126 dev) seems to synthesize a baseline from the bottom margin-edge, for all of the cases in my previous comment, except for the first one where it uses the content-box bottom for the button and input type="color" vs. the margin-box for the others (I think that inconsistency is the thing @bfgeek mentioned as being not correct)

bfgeek commented 6 months ago

See also https://github.com/whatwg/html/issues/5065

I'm not yet entirely sure what makes the most sense here, but one other situation that's worth comparing is what happens when these elements have an orthogonal writing-mode. In Firefox, that makes us synthesize a baseline using their border-edge, it seems.

(Ignoring <input type=color> for reasons) My current patch will synthesize based off the content edges in all of the above cases. (Which seems reasonable?)

bfgeek commented 6 months ago

cc/ @zcorpan

dholbert commented 6 months ago

At a high level, I think the following makes sense based on current specs:

I'm not entirely sure if how-that-baseline-is-synthesized is well-defined for buttons in an inline context, but it seems worth ensuring it's well-defined & consistent.

bfgeek commented 6 months ago

Yeah - that's the same logic I followed as well - as such I believe that:

<button></button> <button style="contain:layout"></button>

https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=12662

Should be aligned (ignoring the issue of how to synthesize baselines for button elements). No browser does this today (likely because of the tests).

I'm going to remove the <button> from one of the tests, and mark the other test as .tentative

frivoal commented 5 months ago

I completely agree with https://github.com/web-platform-tests/wpt/issues/45889#issuecomment-2078048252

I probably agree with https://github.com/web-platform-tests/wpt/issues/45889#issuecomment-2080230139, the nuance being that I don't know which bit of spec says that a button without containment also has no baseline (or alternatively, says that it does have one that happens to coincide with how it gets synthesized if it didn't).