Closed
Bug 1340309
Opened 8 years ago
Closed 8 years ago
[css-align] Implement the updated spec language for interactions between "align-items" & "align-self", "justify-items" & "justify-self"
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: dholbert, Assigned: dholbert)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-needed)
Attachments
(2 files)
59 bytes,
text/x-review-board-request
|
MatsPalmgren_bugz
:
review+
jcristau
:
approval-mozilla-aurora+
jcristau
:
approval-mozilla-beta+
jcristau
:
approval-mozilla-release+
|
Details |
59 bytes,
text/x-review-board-request
|
MatsPalmgren_bugz
:
review+
|
Details |
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•8 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•8 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!)
Updated•8 years ago
|
Blocks: css-align-3
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•8 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.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•8 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.
Updated•8 years ago
|
Assignee: nobody → dholbert
Keywords: dev-doc-needed
Comment 9•8 years ago
|
||
Excellent, this is how I always wanted the static position to work!
Comment 10•8 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•8 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•8 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•8 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•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/06623679bd90
https://hg.mozilla.org/mozilla-central/rev/9ff75c152be3
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 15•8 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.
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+
Comment 16•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/e4f35a45184c
https://hg.mozilla.org/releases/mozilla-aurora/rev/deb6668fecd7
status-firefox53:
--- → fixed
Flags: in-testsuite+
Comment 18•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/ec59d717d479
https://hg.mozilla.org/releases/mozilla-beta/rev/8f3186a4a57b
status-firefox52:
--- → fixed
Comment 19•8 years ago
|
||
bugherder uplift |
Comment 20•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr52/rev/ec59d717d479
https://hg.mozilla.org/releases/mozilla-esr52/rev/8f3186a4a57b
status-firefox-esr52:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•