Skip to content

Commit

Permalink
Bug 1786910: Ignore 'align-content' when determining the aligned stat…
Browse files Browse the repository at this point in the history
…ic position of abspos flex children. r=TYLin a=RyanVM

Before this patch, we honored `align-content` (in combination with
`align-self`) for cross-axis alignment for abspos flex children **in cases
where the flex container was multi-line**. This was a bit weird, but was
required by the spec, and made some sense in the spirit of aligning the abspos
box as if it were the sole flex item in a flex container.

Now the CSSWG has resolved in [1] to simplify things by just ignoring
`align-content` for abspos flex children. So, this patch updates us in
accordance with this change. Such items now only have to look at `align-self`
for cross-axis alignment to determine their static position in their flex
container.

Before this commit, we had a bunch of WPT tests to check the impact of every
align-content value, with all of the various flavors of directionality.  Now
that align-content has *no effect* in any of these cases, all of these tests
become pretty trivial and redundant.  Rather than carrying them all forward
with trivial "no impact" expectations for each scenario, I've just updated the
first/simplest such test to expect no-behavioral-impact and I've removed the
rest.

[1] w3c/csswg-drafts#7596 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D157571
  • Loading branch information
dholbert committed Sep 19, 2022
1 parent f044091 commit 4d1d821
Show file tree
Hide file tree
Showing 15 changed files with 38 additions and 1,377 deletions.
46 changes: 20 additions & 26 deletions layout/generic/nsFlexContainerFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1278,36 +1278,30 @@ StyleAlignFlags nsFlexContainerFrame::CSSAlignmentForAbsPosChild(
StyleAlignFlags alignment{0};
StyleAlignFlags alignmentFlags{0};
if (isMainAxis) {
// We're aligning in the main axis: align according to 'justify-content'.
// (We don't care about justify-self; it has no effect on children of flex
// containers, unless https://github.com/w3c/csswg-drafts/issues/7644
// changes that.)
alignment = SimplifyAlignOrJustifyContentForOneItem(
containerStylePos->mJustifyContent,
/*aIsAlign = */ false);
} else {
const StyleAlignFlags alignContent =
SimplifyAlignOrJustifyContentForOneItem(
containerStylePos->mAlignContent,
/*aIsAlign = */ true);
if (StyleFlexWrap::Nowrap != containerStylePos->mFlexWrap &&
alignContent != StyleAlignFlags::STRETCH) {
// Multi-line, align-content isn't stretch --> align-content determines
// this child's alignment in the cross axis.
alignment = alignContent;
} else {
// Single-line, or multi-line but the (one) line stretches to fill
// container. Respect align-self.
alignment = aChildRI.mStylePosition->UsedAlignSelf(Style())._0;
// Extract and strip align flag bits
alignmentFlags = alignment & StyleAlignFlags::FLAG_BITS;
alignment &= ~StyleAlignFlags::FLAG_BITS;

if (alignment == StyleAlignFlags::NORMAL) {
// "the 'normal' keyword behaves as 'start' on replaced
// absolutely-positioned boxes, and behaves as 'stretch' on all other
// absolutely-positioned boxes."
// https://drafts.csswg.org/css-align/#align-abspos
alignment = aChildRI.mFrame->IsFrameOfType(nsIFrame::eReplaced)
? StyleAlignFlags::START
: StyleAlignFlags::STRETCH;
}
// We're aligning in the cross axis: align according to 'align-self'.
// (We don't care about align-content; it has no effect on abspos flex
// children, per https://github.com/w3c/csswg-drafts/issues/7596 )
alignment = aChildRI.mStylePosition->UsedAlignSelf(Style())._0;
// Extract and strip align flag bits
alignmentFlags = alignment & StyleAlignFlags::FLAG_BITS;
alignment &= ~StyleAlignFlags::FLAG_BITS;

if (alignment == StyleAlignFlags::NORMAL) {
// "the 'normal' keyword behaves as 'start' on replaced
// absolutely-positioned boxes, and behaves as 'stretch' on all other
// absolutely-positioned boxes."
// https://drafts.csswg.org/css-align/#align-abspos
alignment = aChildRI.mFrame->IsFrameOfType(nsIFrame::eReplaced)
? StyleAlignFlags::START
: StyleAlignFlags::STRETCH;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-flexbox/#abspos-items">
<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/7596#issuecomment-1225952646">
<meta charset="utf-8">
<style>
.container {
Expand Down Expand Up @@ -38,10 +39,9 @@
background: teal;
height: 6px;
width: 8px;
/* This "align-self" only gets a chance to take effect when our container
has "align-content: stretch". In that case, it helps us verify that
the container's "align-content: stretch" is actually taking effect
and stretching the flex line (and giving us space to center in). */
/* This "align-self" is actually the only alignment that matters here.
The flex containers' various "align-content" values have no impact on
the positioning of absolutely-positioned flex children. */
align-self: center;
}
</style>
Expand All @@ -57,23 +57,23 @@
<div class="container" style="align-content: normal"><div data-offset-x="2" data-offset-y="3"></div></div>
<br>
<!-- <baseline-position> -->
<div class="container" style="align-content: baseline"><div data-offset-x="2" data-offset-y="1"></div></div>
<div class="container" style="align-content: last baseline"><div data-offset-x="2" data-offset-y="5"></div></div>
<div class="container" style="align-content: baseline"><div data-offset-x="2" data-offset-y="3"></div></div>
<div class="container" style="align-content: last baseline"><div data-offset-x="2" data-offset-y="3"></div></div>
<br>
<!-- <content-distribution> -->
<div class="container" style="align-content: space-between"><div data-offset-x="2" data-offset-y="1"></div></div>
<div class="container" style="align-content: space-between"><div data-offset-x="2" data-offset-y="3"></div></div>
<div class="container" style="align-content: space-around"><div data-offset-x="2" data-offset-y="3"></div></div>
<div class="container" style="align-content: space-evenly"><div data-offset-x="2" data-offset-y="3"></div></div>
<div class="container" style="align-content: stretch"><div data-offset-x="2" data-offset-y="3"></div></div>
<br>
<!-- <content-position>, part 1 -->
<div class="container" style="align-content: center"><div data-offset-x="2" data-offset-y="3"></div></div>
<div class="container" style="align-content: start"><div data-offset-x="2" data-offset-y="1"></div></div>
<div class="container" style="align-content: end"><div data-offset-x="2" data-offset-y="5"></div></div>
<div class="container" style="align-content: start"><div data-offset-x="2" data-offset-y="3"></div></div>
<div class="container" style="align-content: end"><div data-offset-x="2" data-offset-y="3"></div></div>
<br>
<!-- <content-position>, part 2 -->
<div class="container" style="align-content: flex-start"><div data-offset-x="2" data-offset-y="1"></div></div>
<div class="container" style="align-content: flex-end"><div data-offset-x="2" data-offset-y="5"></div></div>
<div class="container" style="align-content: flex-start"><div data-offset-x="2" data-offset-y="3"></div></div>
<div class="container" style="align-content: flex-end"><div data-offset-x="2" data-offset-y="3"></div></div>
<br>
</div>
<div class="small">
Expand All @@ -83,23 +83,23 @@
<div class="container" style="align-content: normal"><div data-offset-x="2" data-offset-y="-1"></div></div>
<br>
<!-- <baseline-position> -->
<div class="container" style="align-content: baseline"><div data-offset-x="2" data-offset-y="1"></div></div>
<div class="container" style="align-content: last baseline"><div data-offset-x="2" data-offset-y="-3"></div></div>
<div class="container" style="align-content: baseline"><div data-offset-x="2" data-offset-y="-1"></div></div>
<div class="container" style="align-content: last baseline"><div data-offset-x="2" data-offset-y="-1"></div></div>
<br>
<!-- <content-distribution> -->
<div class="container" style="align-content: space-between"><div data-offset-x="2" data-offset-y="1"></div></div>
<div class="container" style="align-content: space-between"><div data-offset-x="2" data-offset-y="-1"></div></div>
<div class="container" style="align-content: space-around"><div data-offset-x="2" data-offset-y="-1"></div></div>
<div class="container" style="align-content: space-evenly"><div data-offset-x="2" data-offset-y="-1"></div></div>
<div class="container" style="align-content: stretch"><div data-offset-x="2" data-offset-y="-1"></div></div>
<br>
<!-- <content-position>, part 1 -->
<div class="container" style="align-content: center"><div data-offset-x="2" data-offset-y="-1"></div></div>
<div class="container" style="align-content: start"><div data-offset-x="2" data-offset-y="1"></div></div>
<div class="container" style="align-content: end"><div data-offset-x="2" data-offset-y="-3"></div></div>
<div class="container" style="align-content: start"><div data-offset-x="2" data-offset-y="-1"></div></div>
<div class="container" style="align-content: end"><div data-offset-x="2" data-offset-y="-1"></div></div>
<br>
<!-- <content-position>, part 2 -->
<div class="container" style="align-content: flex-start"><div data-offset-x="2" data-offset-y="1"></div></div>
<div class="container" style="align-content: flex-end"><div data-offset-x="2" data-offset-y="-3"></div></div>
<div class="container" style="align-content: flex-start"><div data-offset-x="2" data-offset-y="-1"></div></div>
<div class="container" style="align-content: flex-end"><div data-offset-x="2" data-offset-y="-1"></div></div>
<br>
</div>
</body>
Expand Down

This file was deleted.

This file was deleted.

Loading

0 comments on commit 4d1d821

Please sign in to comment.