web-platform-tests / wpt

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

Clarification on static position of absolutely positioned children of flex containers #35420

Open rreno opened 1 year ago

rreno commented 1 year ago

@dholbert @davidsgrogan

I am trying to understand the expectations for the static position of an absolutely positioned child of a flex container. As an example css-flexbox/abspos/flex-abspos-staticpos-align-content-005.html expects containers whose align-content is either normal or stretch and whose available space is negative to place their abspos child differently than when align-content is flex-start (lines 57, 67, and 75 respectively).

Reading the spec for align-content, it seems pretty clear that in the case of the .small containers, the align-content: normal and align-content: stretch are to behave as align-content: flex-start. What I don't understand is why then the expected position is different for the divs at lines 57 and 67 and the div at line 75 in the above test.

Could you help me to understand this expectation and what other calculation for position I might be missing? Does it have to do with align-self being used in the positioning for some values of align-content?

davidsgrogan commented 1 year ago

it seems pretty clear that in the case of the .small containers, the align-content: normal and align-content: stretch are to behave as align-content: flex-start

I assume you're referring to this part of the spec? If so, I think you are right.

If the leftover free-space is negative, this value is identical to flex-start.

I suspect the expectations for that test are wrong.

Note there's a big spec issue about changing the abspos behavior, see https://github.com/w3c/csswg-drafts/issues/5843

dholbert commented 1 year ago

So the behavior in that test makes sense based on what the spec used to say. I'm not sure whether it still matches the current spec (it might or might not, I need to read up a bit to remember).

The difference you're noticing isn't about align-content fallback-behavior, but rather it's due to an interaction between align-content and align-self*. See this part of the test where I specifically add align-self:center to trigger this observable interaction: https://github.com/web-platform-tests/wpt/blob/master/css/css-flexbox/abspos/flex-abspos-staticpos-align-content-005.html#L41-L45

Here's a brief explanation/rationalization. The spec used to say to align abspos-flex-children by laying them out as if they were the only flex item, essentially. And in this case, this meant the following for align-content:stretch and normal:

...vs. with align-content:start and flex-start, the flex line is sized to its "flex item" (the abspos flex child), so there's nothing for the child's align-self to do (no space to align within inside of its flex line), which means the child ends up looking start-aligned within the container.

dholbert commented 1 year ago

Quoting myself:

That flex line gets stretched to precisely fill the flex container (per align-content:stretch).

This is backed by this part of the flexbox spec, in the explicit definition of the algorithm:

If the flex container is single-line and has a definite cross size, the cross size of the flex line is the flex container’s inner cross size. https://drafts.csswg.org/css-flexbox-1/#algo-cross-line

@davidsgrogan's quote above ("If the leftover free-space is negative, this value is identical to flex-start.") seems to disagree with this, but I think that quote is from a hand-wavier section (describing the high-level intent of stretch vs. how it actually operates in the algorithm.) I think that hand-wavy quote is accurate for multi-line but not for single-line, per this special-case where stretch does literally just set the cross-size of the flex line to the cross-size of the container.

dholbert commented 1 year ago

er, sorry, I guess in this testcase, the flex container is in fact multi-line, not single-line. (it has flex-flow: column wrap;)

So my quote about stretch doesn't apply after all, since that was specific to single-line. It's possible that part of the spec evolved as well, I faintly recall it may've used to apply to multi-line-flex-containers-that-just-have-one-line...? [EDIT: yes, the spec used to do that. It changed per this proposal]

But yeah, it looks like the test is probably wrong.

davidsgrogan commented 1 year ago

Heh, I also just noticed that the test was multi-line.

rreno commented 1 year ago

Thank you both for your replies! Apologies for not making it clear this is a multi-line container. I suppose it's implicit with align-content to take effect but I've been deep in this recently and it's unlikely others have the same context in their heads as I do right now! I'll try to be more explicit in the future.

So my understanding is that yes, in the case where there is more than one flex line (impossible with abspos children I believe, as they are all given their own, overlapping, flex lines?) in a multi-line container, we should be stretching the lines to fill the available space.

In this case, though, we are considering a single line in a multi-line container and there is negative available space in the container and so we fall back to flex-start.

I think we agree on that final sentence. Is that correct?

davidsgrogan commented 1 year ago

Yes, impossible to have more than one flex line in the context of positioning an abspos child of a flex container.

I agree with that final sentence.

dholbert commented 1 year ago

Yup, agreed with the final sentence.

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1784135 on Firefox's current behavior (i.e. the fact that we "pass" this test with its current incorrect expectations).

rreno commented 1 year ago

I've opened PR #35426 . It looks like I can't request review in the Github UI but I would appreciate you both taking a look. Thanks!

rreno commented 1 year ago

We're tracking various bugs in our treatment of abspos children of flex containers in https://bugs.webkit.org/show_bug.cgi?id=221472

dholbert commented 1 year ago

It looks like a bunch of the neighboring align-self tests are based on the same flawed premise -- e.g. css/css-flexbox/abspos/flex-abspos-staticpos-align-self-002.html

They have align-content unset, so they're getting normal i.e. stretch -- and they're assuming that align-self will have an alignment effect under those conditions (and it does in current Firefox), but in fact it should not, for the reasons discussed here.

bfgeek commented 1 year ago

Are these tests asserting the new static-position behaviour (e.g. https://drafts.csswg.org/css-align-3/#justify-abspos ) or the old static-position behaviour?

dholbert commented 1 year ago

They're asserting the old static-position behavior (align the box just as if it were the sole flex item in the flex container, and use that as the static position). Note that this is still what https://drafts.csswg.org/css-flexbox-1/#abspos-items calls for.

https://github.com/w3c/csswg-drafts/issues/5843 (which @davidsgrogan linked above) is tracking what do do about the disagreement between that somewhat-interoperably-implemented spec text vs. css-align-3's spec text. Presumably when we have a solid resolution on that issue, these tests will want to be updated.

dholbert commented 1 year ago

It looks like a bunch of the neighboring align-self tests are based on the same flawed premise -- e.g. css/css-flexbox/abspos/flex-abspos-staticpos-align-self-002.html

Hmm, it looks like the abspos-staticpos-align-self WPT tests are pretty interoperable right now (all engines at 22/24 passes on most or all of the tests): https://wpt.fyi/results/css/css-flexbox/abspos?label=master&label=experimental&aligned&view=subtest&q=align-self

So given that cross-browser agreement, I worry that sites may have come to expect that align-self will work by default for abspos flex items (i.e. with align-content at its default normal value, which is equivalent to stretch), regardless of whether the flex container happens to be multi-line (flex-wrap:wrap) and too small...

Here's a reduced testcase, which renders with the green box (the abspos thing) centered around the orange box (its flex container) in current Gecko, Blink, and WebKit: https://jsfiddle.net/dholbert/Lsyqcmt0/

.container {
  display: flex;
  flex-wrap: wrap;
  align-items: center;
  justify-content: center;
  margin: 20px;
  width: 30px;
  height: 30px;
  border: 3px solid orange;
  position: relative;
}

.item {
  position: absolute;
  border: 3px solid green;
  height: 50px;
  width: 50px;
}
<div class="container">
<div class="item">
</div>
</div>

I think if we update implementations according to https://github.com/web-platform-tests/wpt/pull/35426 and according to the discussion here, the green box would no longer be centered (i.e. we would no longer honor align-items:center i.e. align-self:center on the abspos thing in that testcase).

Given that this works interoperably right now (even if it's technically against what the tested spec text is trying to call for), it's probably worth stepping back before updating expectations/implementations here...

dholbert commented 1 year ago

Here's a screenshot of how my jsfiddle looks in a Firefox build with a fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1784135 (aimed at keeping up with the test expectation changes in #35426): image

The un-centered position for the green square there is not-great, given the clear align-items:center;justify-content:center markup, and given that all browsers currently center for this example.

(The "new" behavior is technically explainable in terms of a sized-to-content virtual flex line which would normally stretch but falls back to start alignment when there's not enough space; but it's definitely not intuitive and the departure from a currently-interoperable behavior feels risky.)

dholbert commented 1 year ago

So: we may be kind of stuck here.

Given the current interop noted in my past few comments (and in https://github.com/w3c/csswg-drafts/issues/5843#issuecomment-756480046 ), I am guessing that the web already depends on align-self (/ align-items) "Just Working" when align-content is unspecified (and not having any strange nerfing behavior when the flex container is too small and multi-line, even if such nerfing is technically explainable).

The subtests in question here (flex-abspos-staticpos-align-self-* more broadly, and the specific parts of flex-abspos-staticpos-align-content-005.html that you asked about) essentially are testing that this particular currently-interoperable use-case Just Works.

To keep it Just Working, I think we need to either: (1) preserve these tests' expectations, i.e. treat the virtual flex line as negatively-stretching to fill the flex container in this scenario (so that align-self ends up with some nonzero [negative] space to work with for alignment purposes) (2) Just ignore align-content entirely for abspos flex children (that's what all non-Gecko engines do as far as I know, per https://github.com/w3c/csswg-drafts/issues/5843#issuecomment-756480046 ); the test expectations would need an update of course.

...and then of course there's the broader re-thinking of this whole flex abspos alignment stuff in https://github.com/w3c/csswg-drafts/issues/5843 ; I'm not sure where that stands, but that might involve a more-substantial shake-up if use-counters prove that it's possible.

rreno commented 1 year ago

I agree that foo: center all over the place not centering something violates the principle of least surprise. Ignoring align-content for abspos items is indeed what WebKit does currently and based on test results I imagine Blink also ignores that property for these cases.

rreno commented 1 year ago

Since there's some disagreement between what the engines do, none of the engines implement the default value of align-content per the spec, the spec can lead to surprising results, and there's conflict within the spec, I can agree that holding off on any action here is a good plan. Ideally a resolution can be come to in the linked csswg issue.

rreno commented 1 year ago

I filed https://github.com/w3c/csswg-drafts/issues/7596 with the CSSWG to see about aligning spec to currently shipping behavior.