1224634 - [css-grid] Grid items should only stretch the inline direction for justify-self:stretch

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Mats Palmgren (inactive)]

Grid items should only stretch the inline direction for justify-self:stretch
but items that were blockified and became block frames stretch regardless
of justify-self value because that's the default behaviour for block frames
in the inline direction (until they also support the css-align properties).

So, for a grid item with a 'justify-self' value other than 'stretch'
we need to trigger shrink-wrap behavior on its block frame when we
reflow it.

Comment 1

[Mats Palmgren (inactive)]

Attachment: fix

Comment 2

[Mats Palmgren (inactive)]

Attachment: test tweaks

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba6a56d8fb9d

Comment 3

[Daniel Holbert [:dholbert]]

Comment on attachment 8687313 [details] [diff] [review]
fix

Review of attachment 8687313 [details] [diff] [review]:
-----------------------------------------------------------------

Just one nit on the commit message:
> Bug 1224634 - [css-grid] Grid items should only stretch the inline direction for justify-self:stretch. r=dholbert

The application of "only" here is ambiguous & easy to misread.  I initially read this as "only stretch the inline direction [i.e. not block direction]", which confused me.   Really, you mean "only... for justify-self:stretch. [not other justify-self values]"

Please reword to avoid this ambiguity. One possible rewording:
> Bug 1224634 - [css-grid] Make grid items shrink-wrap when
> measuring their size, unless they have justify-self:stretch. r=dholbert

(or whatever you think)

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 8687315 [details] [diff] [review]
test tweaks

Review of attachment 8687315 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/reftests/css-grid/grid-col-max-sizing-max-content-001.html
@@ +17,5 @@
>   grid-auto-columns: minmax(min-content,max-content);
>   border: dashed blue;
>   float:left;
>   clear:left;
> + justify-items: stretch;

Why do we need to add this declaration in all of these tests?

"justify-items: auto" (the default) should compute to "stretch" on a grid already, according to https://drafts.csswg.org/css-align-3/#propdef-justify-items.  Maybe I'm misreading something, though? Or, does that not work correctly right now?

Comment 5

[Mats Palmgren (inactive)]

> Why do we need to add this declaration in all of these tests?

Not really no.  I figured I should have some 'justify-items: stretch' and some
'justify-self: stretch' just for testing.  I'll remove one the declarations here
so we test the 'auto' behavior too.

Comment 6

[Daniel Holbert [:dholbert]]

OK, sounds good.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2906aec76e64
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e66054666ac

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2906aec76e64
https://hg.mozilla.org/mozilla-central/rev/5e66054666ac