https://drafts.csswg.org/css-align-3/#baseline-align-content "Baseline Content-Alignment Grid Items: A grid item participates in first (last) baseline content-alignment in either its row or column (whichever matches its inline axis) if its computed align-content is baseline (last-baseline), and its computed align-self or justify-self (whichever affects its block axis) is stretch or self-start (self-end). For this purpose, the start, end, flex-start, and flex-end values of align-self are treated as either self-start or self-end, whichever they end up equivalent to. "
Here we use the mBaselineOffset established in bug 1221525 to implement *-content baseline alignment. In this case I'm propagating the value in a frame property since it needs to affect the padding we set up in the reflow state. https://drafts.csswg.org/css-align-3/#baseline-align-content "a box participating in baseline content-alignment acts like its padding was increased" The nsHTMLReflowState changes mostly just moves things around since we now need the reflow flags in nsCSSOffsetState::InitOffsets (which is a base class to nsHTMLReflowState where the mFlags lives). The padding contribution is always positive (padding can't be negative) so I'm exploiting that fact by negating the property value for 'last-baseline' to indicate that it's for the end padding.
Attachment #8755944 - Flags: review?(dholbert)
(In reply to Mats Palmgren (:mats) from comment #1) > The nsHTMLReflowState changes mostly just moves things around since > we now need the reflow flags in nsCSSOffsetState::InitOffsets (which > is a base class to nsHTMLReflowState where the mFlags lives). Could most of these nsHTMLReflowState changes be split into a separate non-functional patch? ("Part 0" if you like, or "part 1" with the main changes renamed to "part 2".) Always best to perform non-functional code-moving/refactoring independently from functional changes, where possible.
Comment on attachment 8755944 [details] [diff] [review] part 1 - [css-grid] Implement Grid layout for align|justify-content:baseline|last-baseline Review of attachment 8755944 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the following fixed, though as noted above, I think most* of the nsHTMLReflowState changes should be split into their own no-functional-changes patch. (Only the ApplyBaselinePadding lambda chunk should remain in this main patch, and perhaps the nsCSSOffsetState assertion-text-tweak) ::: layout/generic/nsGridContainerFrame.cpp @@ +4580,5 @@ > } > cb += aContentArea.Origin(wm); > aState.mRows.AlignBaselineSubtree(*aGridItemInfo); > aState.mCols.AlignBaselineSubtree(*aGridItemInfo); > + // Setup [align|justify]-content:[last-]baseline related properties. This probably wants... s/properties/frame properties/ ...to clarify which properties you're talking about here. (since [align|justify]-content are themselves CSS properties.) (This tweak doesn't push this over 80 characters, so no rewrapping needed.) @@ +4585,5 @@ > + // These are added to the padding in nsCSSOffsetState::InitOffsets. > + // (a negative value signals the value is for 'last-baseline' and should be > + // added to the (logical) end padding) > + typedef const FramePropertyDescriptor<SmallValueHolder<nscoord>>* Prop; > + auto SetProp = [aGridItemInfo, aChild] (LogicalAxis aAxis, Consider s/aAxis/aGridAxis/; otherwise, it's ambiguous whether this is about the grid's block-axis/inline-axis vs. the child's grid/inline axis.) ::: layout/generic/nsHTMLReflowState.cpp @@ +161,5 @@ > MOZ_ASSERT(!aFrame->IsFlexOrGridItem(), > "We're about to resolve percent margin & padding " > "values against CB inline size, which is incorrect for " > + "flex/grid items" > + "Additionally for grid items, this path doesn't handle baseline " Right now, this would print out as "...itemsAdditionally..." Please add a period and a space between the word "items" and its close-quote, to fix this.
Attachment #8755944 - Flags: review?(dholbert) → review+
This does the nsHTMLReflowState refactoring separately as you suggested.
Attachment #8755945 - Attachment description: part 2 - [css-grid] Reftests for align|justify-content:baseline|last-baseline and mixed *-content/self tests → part 3 - [css-grid] Reftests for align|justify-content:baseline|last-baseline and mixed *-content/self tests
Try run together with bug 1221525: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba3fa36e865b
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/7294fad22949 part 1 - Move the ReflowStateFlags type to nsCSSOffsetState and add a 'ReflowStateFlags aFlags' param to nsCSSOffsetState::InitOffsets for later use (idempotent patch). r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/7ea32b5e7ecf part 2 - [css-grid] Implement Grid layout for align|justify-content:baseline|last-baseline (aka "Baseline Content-Alignment"). r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/2b4dfcc5160c part 3 - [css-grid] Reftests for align|justify-content:baseline|last-baseline and mixed *-content/self tests.
You need to log in before you can comment on or make changes to this bug.