Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-align] Does stretch work when width/height only behaves as auto? #4525

Closed
Loirooriol opened this issue Nov 21, 2019 · 9 comments
Closed

Comments

@Loirooriol
Copy link
Contributor

Consider this testcase:

<div id="grid">
  <div id="item">Text</div>
</div>
#grid {
  display: grid;
  width: 100px;
  height: 100px;
  grid-template: 100% / 100%;
  background: yellow;
}
#item {
  height: max-content;
  background: cyan;
}

The grid item has height: max-content. As resolved in #2708, this only behaves as auto, but doesn't compute to auto.

It also has the default align-self: normal, which behaves as align-self: stretch for grid items.

From https://drafts.csswg.org/css-align-3/#valdef-justify-self-stretch,

When the box’s computed width/height (as appropriate to the axis) is auto and neither of its margins (in the appropriate axis) are auto, sets the box’s used size to the length necessary to make its outer size as close to filling the alignment container as possible

The quote says "computed height is auto", which technically doesn't hold. So in Chromium, the item is not stretched. But Firefox thinks that the spec actually means "behaves as auto" (which has been a very frequent mistake in the specs), so it stretches the item.

Which is it? I prefer Chromium's behavior, if an author sets height: max-content, I think it's reasonable to assume they desire to size it according to the content, not stretch it to fill the container.

Context: https://bugzil.la/1597055. CC @emilio

@Loirooriol Loirooriol added the css-align-3 Current Work label Nov 21, 2019
@fantasai
Copy link
Collaborator

fantasai commented Jan 9, 2020

I think the intention here is indeed to check the computed value, and that Chromium's behavior is correct. The "behaves as auto" cases that are not computed 'auto' keywords are primarily for sizing behaviors where we can't do the thing requested...

Furthermore, there's an open issue on whether the content-based size keywords should be treated as auto in the general case, see #3973 I think the central mistake there was assuming that 'auto' resolved to "content-based height" as it does in block layout, in which case that would be appropriate... but in Grid and Flexbox that's not the case, and there is special behavior associated with 'auto' that would not be appropriate to trigger if the author specifically wanted to request content-based heights.

@tabatkins
Copy link
Member

I agree that Chrome's behavior is probably better here, but the note in the "Behaves as 'auto'" section seems to imply that all the places where we check for a computed value of "auto" are legacy and should be fixed; if there are valid use-cases for this, we should be a lot more specific about that (maybe, unfortunate as it may be, inventing a new term that specifically means "computed value is auto" for this, just so we can distinguish legacy uses from intentional new uses).

@fantasai
Copy link
Collaborator

fantasai commented Jan 9, 2020

I think we should just audit the specs, fix them where necessary, and limit the note's target to CSS2.

@tabatkins
Copy link
Member

Agenda+ to verify with Emilio (and anyone else who cares) that the Chrome behavior is preferable.

@MatsPalmgren
Copy link

I prefer Chromium's behavior, if an author sets height: max-content, I think it's reasonable to assume they desire to size it according to the content, not stretch it to fill the container.

Seems reasonable to me, fwiw.

@cbiesinger
Copy link

#3973 may change this again

But I guess that still leaves the question on whether this applies to indefinite percentages?

@fantasai
Copy link
Collaborator

@cbiesinger Indefinite percentages don't compute to auto.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Does stretch work when width/height only behaves as auto?, and agreed to the following:

  • RESOLUTION: Keep the spec text about auto and stretching that aligns with Chromium behavior, but add some clarification.
The full IRC log of that discussion <heycam> Topic: Does stretch work when width/height only behaves as auto?
<heycam> github: https://github.com//issues/4525
<heycam> oriol: this is about grid items the default alignment is stretch
<heycam> ... this is supposed to stretch an element to cover tis container
<heycam> ... but not if the computed width computes to auto
<heycam> ... the spec has some ambiguity about computing to auto
<heycam> ... but it's "behaves" as auto
<heycam> ... FIrefox thinks this is the case, and instead of checking the computed value being auto, it checks whether it behaves as auto
<heycam> ... e.g. if you set the height to max-content it behaves as auto, in Firefox it stretches, but in Chromium it doesn't
<RossenF2F> q?
<heycam> ... the question is whether if the width/height is set to a value that behaves as auto but isn't auto itself, does this stretch?
<RossenF2F> q+
<heycam> fantasai: I think the author did not want automatic behavior
<heycam> ... in the most common behavior, automatic behavior is sizing to the content
<RossenF2F> Zakim, open queue
<Zakim> ok, RossenF2F, the speaker queue is open
<heycam> ... in the context of grid and flex, auto isn't equivalent to that
<heycam> ... if the author has set it to a value that's not auto, we shouldn't automatically stretch
<heycam> ... I think the spec is correct to rely on computing to auto, so we should align on Chromium's behavior
<heycam> TabAtkins: mats also says that sounds reasonable
<tantek_> is there a test case for Chromium's behavior?
<heycam> emilio: it's a bit weird that it max-content behaves as auto for some things but not others
<rego> tantek_:
<rego> https://software.hixie.ch/utilities/js/live-dom-viewer/?%3C!DOCTYPE%20html%3E%0A%3Cstyle%3E%0A%23grid%20%7B%0A%20%20display%3A%20grid%3B%0A%20%20width%3A%20100px%3B%0A%20%20height%3A%20100px%3B%0A%20%20grid-template%3A%20100%25%20%2F%20100%25%3B%0A%20%20background%3A%20yellow%3B%0A%7D%0A%23item%20%7B%0A%20%20height%3A%20max-content%3B%0A%20%20align-self%3A%20stretch%3B%0A%20%20background%3A%20cyan%3B%0A%7D%0A%3C%2Fstyle%3E%0A%3Cdiv%20id%3D%22grid%22%3E%0A
<rego> %20%20%3Cdiv%20id%3D%22item%22%3EText%3C%2Fdiv%3E%0A%3C%2Fdiv%3E
<tantek_> IRC broke that
<tantek_> across a line
<rego> yep, linked on the issue
<rego> at the begginig
<rego> https://github.com//issues/4525
<tantek_> thank you
<heycam> RESOLUTION: Keep the spec text about auto and stretching that aligns with Chromium behavior, but add some clarification.
<heycam> astearns: let's get that test case into WPT
<fantasai> s/RESOLUTION/RESOLVED?
<fantasai> s/RESOLUTION/RESOLVED/

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 25, 2020
Per the resolution in w3c/csswg-drafts#4525.

Differential Revision: https://phabricator.services.mozilla.com/D60675

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1597055
gecko-commit: 4eb80ad3235c897dbf240d9042eca9297405ecb1
gecko-integration-branch: autoland
gecko-reviewers: mats
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Jan 25, 2020
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jan 26, 2020
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 27, 2020
Per the resolution in w3c/csswg-drafts#4525.

Differential Revision: https://phabricator.services.mozilla.com/D60675

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1597055
gecko-commit: 4eb80ad3235c897dbf240d9042eca9297405ecb1
gecko-integration-branch: autoland
gecko-reviewers: mats
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Jan 28, 2020
Per the resolution in w3c/csswg-drafts#4525.

Differential Revision: https://phabricator.services.mozilla.com/D60675

UltraBlame original commit: 4eb80ad3235c897dbf240d9042eca9297405ecb1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Jan 28, 2020
Per the resolution in w3c/csswg-drafts#4525.

Differential Revision: https://phabricator.services.mozilla.com/D60675

UltraBlame original commit: 4eb80ad3235c897dbf240d9042eca9297405ecb1
@fantasai
Copy link
Collaborator

fantasai commented Mar 6, 2020

Pushed some clarifications. @Loirooriol @MatsPalmgren Please file issues on anything else that looks suspicious. :) Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants