1715661 - justify-content:left/right in flexbox doesn't work with vertical writing modes

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, defect)

Description

[dgrogan (chrome)]

Attachment: physical-left-right.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.77 Safari/537.36

Steps to reproduce:

Load attached file.

Actual results:

The item in the justify-content:left flexbox (top one) is justified to the right.

The item in the justify-content: right flexbox (bottom one) is justified to the left.

Expected results:

justify-content should be obeyed in a vertical writing mode column flexbox.

https://drafts.csswg.org/css-align-3/#valdef-justify-content-left talks about aligning to the physical left/right edges if possible.

There's a note about column flex containers, but I'm fairly confident it only applies to column flex containers in horizontal writing modes. The note was added in response to https://github.com/w3c/csswg-drafts/issues/2991, which explicitly called out horizontal writing modes.

Comment 1

[dgrogan (chrome)]

+aethanyc for a look

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Layout: Flexbox' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

In your testcase, the flex container is column based with vertical writing-mode. That is, the main axis is physical left↔right axis. I agree with you per spec, justify-content:left and justify-content:right should follow the physical left / right if possible.

The physical left/right was added to the spec in https://github.com/w3c/csswg-drafts/commit/93ee87ca370963205c93c915b2b730c3a308a142. However, it appears that no browser has implemented the resolution, and all the engines still fall back to justify-content:start in your testcase. So we are still interoperable.

Does blink plan to change the behavior in FlexNG? If so, could you provide the link to your bug? If we were to change the behavior, it's better to coordinate the release roughly at the same time to reduce the confusion to web developers.

To Firefox developers: this part of the code makes justify-content always be start in a column flex container.
https://searchfox.org/mozilla-central/rev/b52cf6bbe214bd9d93ed9333d0403f7d556ad7c8/layout/generic/nsFlexContainerFrame.cpp#3383-3386

Comment 4

[Daniel Holbert [:dholbert]]

Yeah, looks like our implementation is correct according to the spec just before the github commit that TYLin referenced. (The person who implemented this did actually have a patch that was closer to the current spec, but I pushed back and had it adjusted to match the spec-at-the-time, at the end of bug 1221565 comment 5. And then the spec later changed. :) Ah well.)

When we fix this, we'll likely want a similar fix for align-self and align-content.

Comment 5

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

When we fix this, we'll likely want a similar fix for align-self and align-content.

We don't need to fix align-self and align-content. Per resolution in #1403, left and right are removed from align-self and align-content. The spec is edited in https://github.com/w3c/csswg-drafts/commit/21a4728f9a044cd202b094809c059295cee192f1.

Good news that our CSS parser only accepts left and right for justify-* (inline-axis) in align.rs, but not align-* (block-axis).

Comment 6

[dgrogan (chrome)]

looks like our implementation is correct according to the spec just before the github commit that TYLin referenced

Heh, yeah, I was wondering if maybe this behavior was specified after your implementation.

we'll likely want a similar fix for align-self and align-content

left/right only apply to justify-*, so you're off the hook for align-self and align-content

Does blink plan to change the behavior in FlexNG? If so, could you provide the link to your bug?

Yeah, I'm hoping to land the change in Chrome 93, which will hit stable channel ~September 1st. I have no data but I'm not really worried about breaking sites with this change. I don't think there are many vertical flex containers out there in the first place*, and there are surely very few that also have justify-content: left/right.

*vertical writing mode flex containers were somewhat just plain broken in Chrome before FlexNG. But we had very few bug reports about them, possibly even 0. So I infer that they aren't used very much.

But, if you've seen other evidence that makes you concerned about breakage and it would be difficult to change Firefox behavior around the same time, I can probably just delay our behavior change by putting the Blink impl behind a flag. Let me know.

Blink Bug is https://crbug.com/1011718. Though that bug is actually for implementing all of right/left/start/end/self-start/self-end in flexbox, which we don't currently support. I noticed the physical-left-right bug while adding flex support for those keywords and checking Blink's new behavior against Firefox.

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

I don't worry too much about breaking sites, either. Vertical flex container and justify-content: left/right sounds like a edge case that probably only browser implementers care about ;) So +1 on make this case correct in your new implementation without putting it behind any flag.

Unless Daniel feel otherwise, we'll wait for https://crbug.com/1011718 to close before changing our behavior. I believe you probably plan to add a WPT test or update existing WPT tests to match the latest spec. When the change is imported into gecko, we'll know.

David, thanks again for the heads-up :)

Comment 8

[Daniel Holbert [:dholbert]]

Yeah, I agree this isn't a strong compat concern; I think we could fix this any time (now or later) without worrying too much about it; whenever someone's got cycles available to do so.

Comment 9

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1715661 - Fix justify-content:{left|right} for column oriented vertical flex container. r?dholbert

In a column oriented vertical flex container, its main axis is parallel
to physical left <-> right axis, so justify-content:left and
justify-content:right should map to physical left and right edge,
respectively. This patch makes this particular case go to the (tweaked)
else branch.

https://drafts.csswg.org/css-align-3/#valdef-justify-content-left

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df5aed89a479
Fix justify-content:{left|right} for column oriented vertical flex container. r=dholbert

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/df5aed89a479