[css-align] Implement the updated spec language for interactions between "align-items" & "align-self", "justify-items" & "justify-self"

RESOLVED FIXED in Firefox 52

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks 1 bug, {dev-doc-needed})

Trunk
mozilla54
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox52 fixed, firefox-esr52 fixed, firefox53 fixed, firefox54 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

2 years ago
It sounds like the CSSWG is revisiting a change to how "align-self"/"align-items" work, per https://github.com/w3c/csswg-drafts/issues/440#issuecomment-280459999 .     

This probably means we may need to adjust the changes that we made in bug 1304012 and/or bug 1269046.  I'm not clear what the updated spec text will be, so I don't yet know what'll need to change -- but I'm filing this as a placeholder bug right now, for whatever changes are coming in that github-issue.
Assignee

Comment 1

2 years ago
So right now the spec text is the following (I've added some minor reformatting for clarity):

 # The 'auto' keyword is interpreted as
 # (1) 'normal', if the box is absolutely positioned or has no parent.
 # (2) otherwise, as the computed value of 'align-items' on the parent.
https://drafts.csswg.org/css-align/#valdef-align-self-auto
https://drafts.csswg.org/css-align/#valdef-justify-self-auto

From chatting briefly with fantasai/tab in IRC, I believe the plan is to change this to make the logic a bit more subtle -- so that some abspos cases (when the static position is being used for positioning) will actually use the (2) interpretation rather than the (1) interpretation.
Assignee

Comment 2

2 years ago
(which means fortunately we're not changing the computation behavior at all -- all that's changing is how-to-react-to-"auto"-in-layout. So, we won't need to revert anything from bug 1304012. hooray!)
Blocks: css-align
OS: Unspecified → All
Hardware: Unspecified → All
Summary: Implement the updated spec language for interactions between "align-items" & "align-self", "justify-items" & "justify-self" → [css-align] Implement the updated spec language for interactions between "align-items" & "align-self", "justify-items" & "justify-self"
Comment hidden (obsolete)
Assignee

Comment 4

2 years ago
Sorry, I skimmed and misread - comment 3 is quoting an old spec change, not the new spec change.  The new spec text is still coming here.
Assignee

Updated

2 years ago
Duplicate of this bug: 1330990
Assignee

Comment 8

2 years ago
The final spec text isn't available here yet, but I'm fairly sure this is what it'll change to

See IRC discussion here:
  https://log.csswg.org/irc.w3.org/css/2017-02-16/#e774476

Note that the code I'm changing -- CSSAlignmentForAbsPosChild -- is only invoked when determining static position, in the "OffsetToAlignedStaticPos" function, here:
https://dxr.mozilla.org/mozilla-central/rev/5069348353f8fc1121e632e3208da33900627214/layout/generic/nsAbsoluteContainingBlock.cpp#449-450

So this change only affects our calculation of the static position.
Assignee: nobody → dholbert
Keywords: dev-doc-needed
Excellent, this is how I always wanted the static position to work!

Comment 10

2 years ago
mozreview-review
Comment on attachment 8840945 [details]
Bug 1340309 part 1: Resolve "align-self:auto"/"justify-self:auto" to flex/grid parent's *-items value, when resolving static pos of abspos children.

https://reviewboard.mozilla.org/r/115330/#review117050
Attachment #8840945 - Flags: review?(mats) → review+

Comment 11

2 years ago
mozreview-review
Comment on attachment 8840946 [details]
Bug 1340309 part 2: Adjust some abspos align-self/justify-self tests to better exercise "auto" value.

https://reviewboard.mozilla.org/r/115332/#review117052
Attachment #8840946 - Flags: review?(mats) → review+

Comment 12

2 years ago
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06623679bd90
part 1: Resolve "align-self:auto"/"justify-self:auto" to flex/grid parent's *-items value, when resolving static pos of abspos children. r=mats
https://hg.mozilla.org/integration/autoland/rev/9ff75c152be3
part 2: Adjust some abspos align-self/justify-self tests to better exercise "auto" value. r=mats
Assignee

Comment 13

2 years ago
Comment on attachment 8840945 [details]
Bug 1340309 part 1: Resolve "align-self:auto"/"justify-self:auto" to flex/grid parent's *-items value, when resolving static pos of abspos children.

Might be too late for beta uplifts, but I'm requesting it just in case. This is definitely something we should get into ESR52, in my opinion, and it'd be nice to get it into the main 52 release as well, because otherwise this manifests as a regression from a user's perspective. (E.g. bug 1330990, bug 1341560)

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1269046 (which implemented a pile of spec changes).  Other browsers (Chrome/Edge) never implemented this one *particular* spec-change (some special handling of "align-self:auto" on abspos elements), and now the spec is reverting to match those other browsers -- so we'll be the odd one out, even though we were technically implementing the spec correctly.
[User impact if declined]: webcompat pain (sites breaking in newer Firefox versions while still working in other browsers) E.g. bug 1330990, bug 1341560
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: No, but it will be soon. I've verified it in local builds.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]:
 - It's very edge-casey (only makes a difference for elements which are both absolutely positioned and the child of a flex or grid container)
 - This is a region of code where we've been incompatible with other browsers for quite a while (until Bug 1269046 was fixed), so sites haven't been able to depend on this behavior here too much in the past. This change does represent one area where our old behavior *was* interoperable with other browsers, and this change is restoring that interoperability.
[String changes made/needed]: None
Attachment #8840945 - Flags: approval-mozilla-beta?
Attachment #8840945 - Flags: approval-mozilla-aurora?

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/06623679bd90
https://hg.mozilla.org/mozilla-central/rev/9ff75c152be3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8840945 [details]
Bug 1340309 part 1: Resolve "align-self:auto"/"justify-self:auto" to flex/grid parent's *-items value, when resolving static pos of abspos children.

last minute webcompat fix for 52

Should be in 52 rc2
Attachment #8840945 - Flags: approval-mozilla-release+
Attachment #8840945 - Flags: approval-mozilla-beta?
Attachment #8840945 - Flags: approval-mozilla-beta+
Attachment #8840945 - Flags: approval-mozilla-aurora?
Attachment #8840945 - Flags: approval-mozilla-aurora+
Assignee

Updated

2 years ago
Duplicate of this bug: 1341560
You need to log in before you can comment on or make changes to this bug.