Bug 1306213: When resolving a flex item's "align-self: auto", use the flex container (not style-context parent) as the "align-items" source.
58 bytes, text/x-review-board-request
As noted in bug 1304012 comment 31, we resolve "align-self:auto" against the parent style-context right now, via this code (where |this| is a FlexItem): mAlignSelf = aFlexItemReflowInput.mStylePosition->UsedAlignSelf( mFrame->StyleContext()->GetParent()); https://dxr.mozilla.org/mozilla-central/rev/66a77b9bfe5dcacd50eccf85de7c0e7e15ce0ffd/layout/generic/nsFlexContainerFrame.cpp#1720 *Usually* this means we resolve it against the flex container. But for tables, the flex item will be the nsTableWrapperFrame, whose parent style-context is the table's style context (rather than the flex container's). So we end up misrendering the align-self:auto chunks of https://dxr.mozilla.org/mozilla-central/source/layout/reftests/w3c-css/submitted/flexbox/flexbox-align-self-horiz-001-table.xhtml (At least, we do after we've applied bug 1304012's fixes. I'm not quite sure why we don't misrender them right now; I think our align-self inheritance hacks must save us somehow, but I need to think through it a bit further.) Anyway -- on a flex item, clearly the intent is that "align-self:auto" should resolve (at used-value-time) to the flex container's align-items value, regardless of style-context parentage. Filing this bug on fixing that.
Comment on attachment 8796035 [details] Bug 1306213: When resolving a flex item's "align-self: auto", use the flex container (not style-context parent) as the "align-items" source. https://reviewboard.mozilla.org/r/82034/#review80626
Attachment #8796035 - Flags: review?(mats) → review+
Thanks! I triggered a Try run; I'll land after that completes, if there are no surprises.
Try is looking good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=84b9f6e010b9 (opt testrun is done, debug testrun is mostly-done) I'll optimistically assume the rest of the test runs will be fine, and land.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/d40e2abb42d6 When resolving a flex item's "align-self: auto", use the flex container (not style-context parent) as the "align-items" source. r=mats
(In reply to Daniel Holbert [:dholbert] from comment #0) > So we end up misrendering the align-self:auto chunks of [...] flexbox-align-self-horiz-001-table.xhtml > (At least, we do after we've applied bug 1304012's fixes. I'm not quite sure > why we don't misrender them right now; I think our align-self inheritance > hacks must save us somehow, but I need to think through it a bit further.) Ah, right -- it makes sense that this works "correctly" in current Nightly (before the patch here). Basically, it works because we resolve the value earlier on (in nsRuleNode) and we never end up actually using the passed-in nsStyleContext, for nsTableWrapperFrame. That's because nsTableWrapperFrame has "align-self:inherit", and we have a special case to handle "align-self:inherit inheriting auto" (which clones align-items from the grandparent), in a special case within nsRuleNode.cpp. (That special case is getting removed in bug 1304012.) So in current trunk, our nsTableWrapperFrame-flex-item has its StylePosition()->mAlignSelf **already resolved** to some non-"auto" value (via the nsRuleNode.cpp inheritance code which aggressively resolves it when it detects that we're inheriting "auto"). So it doesn't actually matter which parent-style-context we pass to the "UsedAlignSelf()" method (technically called ComputedAlignSelf() in current trunk), because that style context is not needed/used at all. But after bug 1304012, we just let "auto" be inherited as itself, so we *do* end up needing & using the passed-in nsStyleContext in the UsedAlignSelf() method. And it needs to be the nsStyleContext for the flex container, as noted above.
You need to log in before you can comment on or make changes to this bug.