Closed Bug 1330990 Opened 3 years ago Closed 3 years ago

flexbox behavior change in Firefox 52 around "align-self:auto" on abspos children of flex container

Categories

(Core :: Layout, defect)

52 Branch
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1340309
Tracking Status
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed

People

(Reporter: soeren.hentzschel, Unassigned, NeedInfo)

References

Details

(Keywords: regression, site-compat)

Attachments

(1 file)

Attached file bug.html
[Tracking Requested - why for this release]:

Either bug 1269045 or bug 1269046 introduced a flexbox regression in Firefox 52:

22:29.52 INFO: Oh noes, no (more) inbound revisions :(
22:29.52 INFO: Last good revision: b48820f4c87aeaa2186c401c1bafe6661d6b7e31
22:29.52 INFO: First bad revision: 995211b23a553e1518e8c79570d27f5c4fd0bb86
22:29.52 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b48820f4c87aeaa2186c401c1bafe6661d6b7e31&tochange=995211b23a553e1518e8c79570d27f5c4fd0bb86

I attached a reduced test case. The word "bug" should be vertical aligned but is on the top in Firefox Developer Edition 52 and Firefox Nightly 53. It works as expected in Firefox 50.1, Firefox Beta 51, Chrome and Safari.
Keywords: regression
Keywords: site-compat
So this is about what "align-self:auto" (the initial value) does on that abspos child -- and whether or not it co-opts the "align-items:center" value from the parent.

You're expecting that it should co-opt that value (understandably so, because that's how it works for non-abspos flexbox children) -- and the spec *used* to say that it should, but now it says that it should *not*.

Quote:
> The 'auto' keyword is interpreted as 'normal' if the box is absolutely positioned or has no parent,
> and as the computed value of 'align-items' on the parent otherwise.
https://drafts.csswg.org/css-align/#propdef-align-self

So for this abspos child, we take the first option there and behave as if we had "align-self:normal" on this child.  And further down, the spec says that (for the purposes of computing the static position of an abspos element):
> 'normal': Behaves as (the fallback for) stretch (because it is treated as fixed-size).
https://drafts.csswg.org/css-align/#align-abspos-static

...and elsewhere "the fallback for stretch" is defined to be "flex-start", here:
 https://drafts.csswg.org/css-align/#valdef-justify-self-stretch says 

So, we're matching the spec here.  Other browsers do not, because this abspos behavior is a relatively recent spec change.

From a "how do I make my site work" perspective, you can work around this inconsistency by setting "align-self" directly on the abspos thing itself.  The point of the new spec language is that abspos things don't care much about their environment -- they eventually will align the same way regardless of their container, I think. (And for now, they'll pretty much align the same way in grid/flex regardless of their container.)
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INVALID
This spec language was clarified here BTW:
 https://github.com/w3c/csswg-drafts/issues/440

(Even at the time that I filed that issue, the spec called for our current behavior -- as noted in the title of that issue report, it said "align-self: auto behaves as start" which is what we're doing here.)

And I filed https://bugs.chromium.org/p/chromium/issues/detail?id=661662 on this issue in Chrome.
Thanks for clarification and the pointer to the chromium ticket!
Duplicate of this bug: 1341560
I guess we should document this anyway. site-compat doc coming.
Actually -- the spec is changing *again* here (after some discussion on https://github.com/w3c/csswg-drafts/issues/440 in the last month), and after the change, the reporter's expected behavior will be what the spec actually calls for. :)

I filed Bug 1340309 on implementing the new spec-text in Firefox, when it appears.  I guess I'll forward-dupe this to that bug.
Resolution: INVALID → DUPLICATE
Summary: flexbox regression in Firefox 52 → flexbox behavior change in Firefox 52 around "align-self:auto" on abspos children of flex container
Duplicate of bug: 1340309
Daniel, so it looks like Chrome's issue is no longer an issue now based on *latest* change on css-align spec. Given that current Firefox 52 behavior is different from other browsers, would you consider make the change in 52 beta cycle(2 weeks left) ? I'm concerning the coming webcompat issues once 52 goes to release cycle.
Flags: needinfo?(dholbert)
Yeah, we already have one dup here, might be better to back out the original change before 52 hits the streets.

[Tracking Requested - why for this release]: A site compatibility issue.
(In reply to Kohei Yoshino [:kohei] from comment #8)
> might be better to back out the original
> change before 52 hits the streets.

I don't think that'd be wise.

The original change (bug 874718 basically) is too large to backout at this point -- and it addressed more web compat issues than it caused (see its large dupe list) by bringing us mostly into alignment with Chrome & Edge on other aspects of abspos flex children.  Though it did make us diverge from them on this one aspect (per spec).

Unfortunately the forthcoming *updated* spec text (from comment 6) isn't written yet.  So we're still matching the latest spec text here, but not (quite) matching other browsers & not matching expected-future-spec-text.

But fortunately, it should be pretty easy to implement the likely-coming spec text here. I'll do my best to do that over in dupe-target bug 1340309, and we can try to get it uplifted before release.  I'm on PTO but I hopefully can crank out a patch tomorrow and we can try to get it uplifted next week.
There isn't "two weeks left" for 52.  We're building the first release candidate next Monday.
In that case, it's likely too late to make a functional change here.

The change I *think* we need is pretty trivial. But the CSSWG hasn't actually discussed the expected change yet -- I've just gotten a rough overview from the spec editors.  So, I'd be uneasy landing an unsettled spec-change for this and shipping it immediately.

So, I think we'll need to leave things as they are for Firefox 52, and get the forthcoming spec-text implemented in Firefox 53, and maaaybe uplift into 52.0.1 if there's enough compat pain and if the final specced change is as trivial as I think it'll be.
Flags: needinfo?(dholbert)
(In reply to Daniel Holbert [:dholbert] (vacation, returning 2/27) from comment #11)
> So, I think we'll need to leave things as they are for Firefox 52, and get
> the forthcoming spec-text implemented in Firefox 53, and maaaybe uplift into
> 52.0.1 if there's enough compat pain and if the final specced change is as
> trivial as I think it'll be.

Thanks Daniel. I think you are suggesting this bug is won't-fix in current beta 52 cycle.
You probably wanna update status flag as well.
Btw, it seems dev-doc for flex-box and 52 release note all need to be updated accordingly.
Do you have idea whether it's being done or not ?
Flags: needinfo?(dholbert)
(In reply to Astley Chen [:astley] (UTC+8) from comment #12)
> Thanks Daniel. I think you are suggesting this bug is won't-fix in current
> beta 52 cycle.
> You probably wanna update status flag as well.

This bug here is a duplicate, so it shouldn't really have status flags.

And as for its dupe-target, I'm not really sure it makes sense to mark it as "52:wontfix" because we *might* want to take a fix in a 52.0.1 if the spec is ready soon enough and/or if there's enough compat pain.

> Btw, it seems dev-doc for flex-box and 52 release note all need to be
> updated accordingly.
> Do you have idea whether it's being done or not ?

Looks like https://developer.mozilla.org/en-US/docs/Web/CSS/align-self does indeed need an update to keep up with the spec. And https://developer.mozilla.org/en-US/Firefox/Releases/52 ("Firefox 52 for web developers") probably needs a mention of bug 1269046. I don't think this change merits a larger user-facing release note, though.

Leaving needinfo open to remind myself to make those changes.
You need to log in before you can comment on or make changes to this bug.