[webkit-reviews] review granted: [Bug 110341] CSS Flexbox: dynamically applied align-items doesn't affect item alignment : [Attachment 190336] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Feb 26 12:00:55 PST 2013
Ojan Vafai <ojan at chromium.org> has granted Tony Chang <tony at chromium.org>'s
request for review:
Bug 110341: CSS Flexbox: dynamically applied align-items doesn't affect item
alignment
https://bugs.webkit.org/show_bug.cgi?id=110341
Attachment 190336: Patch
https://bugs.webkit.org/attachment.cgi?id=190336&action=review
------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=190336&action=review
> Source/WebCore/ChangeLog:10
> + The problem was with flex items that were previously stretch but
didn't change width,
> + we would not lay them out again. Since they were not layed out, we
use the old stretch
> + value. Fix this by marking flex items that were stretch as needing
layout.
This grammar is a bit broken. I see what you're saying, but might be worth
cleaning up.
> Source/WebCore/rendering/RenderFlexibleBox.cpp:316
> + if (oldStyle && oldStyle->alignItems() == AlignStretch && diff ==
StyleDifferenceLayout) {
> + for (RenderBox* child = firstChildBox(); child; child =
child->nextSiblingBox()) {
> + EAlignItems previousAlignment = resolveAlignment(oldStyle,
child->style());
> + if (previousAlignment == AlignStretch && previousAlignment !=
resolveAlignment(style(), child->style()))
> + child->setChildNeedsLayout(true, MarkOnlyThis);
> + }
> + }
I think this is correct, but it's tricky enough that it could use a brief "why"
comment, e.g. why we only care about the alignment going from stretch to
something else.
More information about the webkit-reviews
mailing list