flex-align landed before flex-flow: column, so there are probably some bugs here. We also need tests.
*** Bug 70980 has been marked as a duplicate of this bug. ***
Created attachment 112618 [details] testcase
Created attachment 112963 [details] Patch
This patch needs on the patch in bug 71161 to pass the new tests.
Comment on attachment 112963 [details] Patch Attachment 112963 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10240240
Comment on attachment 112963 [details] Patch Attachment 112963 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/10237433
Comment on attachment 112963 [details] Patch Attachment 112963 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/10244120
Comment on attachment 112963 [details] Patch Attachment 112963 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10238440
Comment on attachment 112963 [details] Patch Attachment 112963 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/10238503
Created attachment 112977 [details] Patch
Created attachment 113001 [details] Patch
*** Bug 70774 has been marked as a duplicate of this bug. ***
Comment on attachment 113001 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113001&action=review > Source/WebCore/ChangeLog:13 > + * rendering/RenderBox.cpp: > + (WebCore::RenderBox::sizesToIntrinsicLogicalWidth): > + When flexitems are column, they should size to the intrinsic width unless flex-flow is stretch. Nit: Maybe explain why this is different from the old flexbox code. Maybe we can get rid of the boxOrient and boxAlign checks on the old flexbox code? > Source/WebCore/rendering/RenderFlexibleBox.cpp:733 > else If we change the width, do we have to relayout the flexitem? Maybe not since it's only growing beyond the intrinsic size.
(In reply to comment #13) > (From update of attachment 113001 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=113001&action=review > > > Source/WebCore/ChangeLog:13 > > + * rendering/RenderBox.cpp: > > + (WebCore::RenderBox::sizesToIntrinsicLogicalWidth): > > + When flexitems are column, they should size to the intrinsic width unless flex-flow is stretch. > > Nit: Maybe explain why this is different from the old flexbox code. Maybe we can get rid of the boxOrient and boxAlign checks on the old flexbox code? I tried removing the boxOrient and boxAlign checks in the old flexbox codepath and it broke a bunch of tests. I think the old spec didn't require everything to be shrinkwrapped.
Comment on attachment 113001 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113001&action=review >>> Source/WebCore/ChangeLog:13 >>> + When flexitems are column, they should size to the intrinsic width unless flex-flow is stretch. >> >> Nit: Maybe explain why this is different from the old flexbox code. Maybe we can get rid of the boxOrient and boxAlign checks on the old flexbox code? > > I tried removing the boxOrient and boxAlign checks in the old flexbox codepath and it broke a bunch of tests. I think the old spec didn't require everything to be shrinkwrapped. Yeah, I don't actually know the old code well enough to know why it doesn't require everything to be shrinkwrapped.
(In reply to comment #13) > (From update of attachment 113001 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=113001&action=review > > > Source/WebCore/rendering/RenderFlexibleBox.cpp:733 > > else > > If we change the width, do we have to relayout the flexitem? Maybe not since it's only growing beyond the intrinsic size. As discussed in person, there seem to definitely be cases where we need to do another layout: -when changing the width -if there are positioned items positioned wrt the bottom/right of the element Possibly others. Definitely a separate patch. Maybe this calls for making the default value for flex-align not being stretch.
Comment on attachment 113001 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=113001&action=review r=me > Source/WebCore/rendering/RenderBox.cpp:1811 > +#if ENABLE(CSS3_FLEXBOX) > + if (parent()->isFlexibleBox()) > + return true; > +#endif Put a FIXME in here that the stretching children of vertical boxes will end up doing an extra layout with this approach.
Committed r100365: <http://trac.webkit.org/changeset/100365>