When a flexible layout box element has a width that is much smaller than the width required by its children, some child boxes may be shrunk past their min-width, even to width:0. Firefox respects content min-width. For more info and an example: http://oli.jp/bugs/webkit/flexbox-flex-shrinking.html Checked in WebKit Version 5.0.3 (6533.19.4, r75772) and Chrome 8.0.552.237
Created attachment 91597 [details] Patch
Hyatt, can you review this? I'm going to be working with Ojan on the new flexbox spec, but wanted to start off by fixing some old flexbox bugs.
Comment on attachment 91597 [details] Patch Further testing shows that this regressed some other tests (a few form controls use -webkit-box and they're not clipping properly).
Created attachment 91766 [details] Patch
Comment on attachment 91766 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91766&action=review > LayoutTests/platform/chromium-win/media/audio-delete-while-slider-thumb-clicked-expected.txt:-5 > -timeupdate This seems unrelated. > Source/WebCore/rendering/RenderBlock.cpp:4521 > + if (!isTableCell() && !isFlexibleChild() && style()->logicalWidth().isFixed() && style()->logicalWidth().value() > 0) > m_minPreferredLogicalWidth = m_maxPreferredLogicalWidth = computeContentBoxLogicalWidth(style()->logicalWidth().value()); > else { > m_minPreferredLogicalWidth = 0; I guess I don't understand why flex children want no preferrered logical width.
Comment on attachment 91766 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91766&action=review >> LayoutTests/platform/chromium-win/media/audio-delete-while-slider-thumb-clicked-expected.txt:-5 >> -timeupdate > > This seems unrelated. This change happened in http://trac.webkit.org/changeset/78562 for the other ports, but that change caused this test to timeout in Chromium. Now that the test doesn't timeout anymore, the chromium specific result needs the change too. >> Source/WebCore/rendering/RenderBlock.cpp:4521 >> m_minPreferredLogicalWidth = 0; > > I guess I don't understand why flex children want no preferrered logical width. It still has a preferred logical width, it's just computed (see below the call to computeBlockPreferredLogicalWidths) rather than just using the provided css width. What this is saying is, "if a flex child has a width specified, allow it to shrink below this width when applying the shrinking algorithm". An example of this is the 4th case in the layout test. In that test, the flexbox has a width of 300px and there are three children each with a width of 200px. The children should shrink below 200px, but instead, they were overflowing.
Created attachment 91939 [details] Patch
(In reply to comment #7) > Created an attachment (id=91939) [details] > Patch Rebased so it can run on the ews bots.
Comment on attachment 91939 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91939&action=review I'd strongly recommend just saving algorithmic changes for building a brand new version of flexbox that matches the spec, and leave this deprecated legacy version alone. > Source/WebCore/rendering/RenderBlock.cpp:629 > +bool RenderBlock::isFlexibleChild() > +{ > + return parent() && parent()->isFlexibleBox() && !isPositioned() && style()->boxFlex() > 0.0f; > +} I know they don't allow it right now, but just to be forward-thinking, I'd make it !isFloatingOrPositioned instead of just !isPositioned(). Also, this seems incomplete to me. What about flexing replaced elements? Seems like all you're fixing here is RenderBlock children of a flexbox, but there can be other types of children like images or replaced elements. > Source/WebCore/rendering/RenderFlexibleBox.cpp:1055 > + if (child->style()->minWidth().isFixed() && child->scrollsOverflowX()) I don't completely understand this case. I don't get why we'd ignore the minWidth of the object when you don't scroll. I'd need to see the latest flexbox draft to see what they suggest, but this seems suspicious to me.
Not planning on fixing bugs in the old flexbox.