Created attachment 192522 [details] Testcase See the attached test case and scroll down. See the empty space being allotted for the image but not being used? The image should not extend off the top into the area that can't be seen. The behavior as-is really only makes sense with overflow: visible. Is this a problem with WebKit or a problem with the spec? <rdar://problem/13392441>
<rdar://problem/13392908>
*** Bug 112048 has been marked as a duplicate of this bug. ***
This is how the spec is defined. center is supposed to distribute space equally between the top and the bottom, so we overflow off the top. If you want the more common behavior of "safe" centering (if there's overflow, only overflow in the scroll direction only), you can set the margin top and margin bottom to auto. It's not clear to me if that works for what you're trying to do here since that doesn't produce a scroll bar. What behavior do you want?
To clarify safe centering: You set the margin top and margin bottom to auto on the img (flex item).
Also, it looks like on Chrome 26, there's no extra space below the image, but in Chrome 27 and ToT there is. That seems like a regression.
Even in Chrome 26, we do true centering, which is kind of weird when overflow is not visible. I think we should propose changing the spec to only do true centering for overflow:visible.
I think it only makes sense for overflow: visible. I will try margin: auto in my original content for now. Thanks!
FWIW, the deprecated flexbox smartly centers in overflow: auto/scroll.
<rdar://problem/13393535>
Can you give a use-case for using center alignment and overflow:scroll/auto? A use-case would be helpful for making an argument on www-style.
The use case is to just center an image in the container when it is small. When it is large overflow where you can still see the whole image by scrolling down. Using margin: auto does work, so I'm fine with that. If the spec isn't changes then -webkit-align-items: center causing dead whitespace should be fixed. Right now it is weird you can only see the whole image if you use flex-start or stretch. The other keywords can hide content off the top and still allow you to scroll to see content at the end. Thinking about it logically, I think flex-end should set the scroll position and force you to scroll up to see the overflow. Same could be said for center, force the scroll position to be centered and you can scroll up or down in the overflow. The initial visual display is just a clipped version of the overflow: visible result. Does that make sense? I'm not sure if that would fly with the spec editors though…
I'm going to focus this bug on fixing the extra whitespace being allotted for the image. We can file a new bug if the spec gets updated and we want to special case centering based on overflow.
Created attachment 192844 [details] Patch
Comment on attachment 192844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192844&action=review > Source/WebCore/rendering/RenderFlexibleBox.cpp:442 > + if (child->isOutOfFlowPositioned()) > + continue; Maybe add a test-case with out of flow positioned children? Or does one of the existing test cases already cover this? > Source/WebCore/rendering/RenderFlexibleBox.cpp:444 > + LayoutUnit childLogicalBottom = logicalTopForChild(child) + logicalHeightForChild(child) + marginAfterForChild(child); > + maxChildLogicalBottom = std::max(maxChildLogicalBottom, childLogicalBottom); Technically, we only need to do this if the child is aligned center or flex-end, but I suppose it's not worth the extra code.
Comment on attachment 192844 [details] Patch Clearing flags on attachment: 192844 Committed r145736: <http://trac.webkit.org/changeset/145736>
All reviewed patches have been landed. Closing bug.
(In reply to comment #14) > (From update of attachment 192844 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192844&action=review > > > Source/WebCore/rendering/RenderFlexibleBox.cpp:442 > > + if (child->isOutOfFlowPositioned()) > > + continue; > > Maybe add a test-case with out of flow positioned children? Or does one of the existing test cases already cover this? Sure, https://bugs.webkit.org/show_bug.cgi?id=112294 has a test case for this.