in a column flexbox, input overflows the box when stretched

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Tony Chang]

Example in the URL.  On Chromium Mac, the input box overflows the flexbox.

[Tony Chang]

Created attachment 168796 [details]
Patch

[Darin Adler]

Comment on attachment 168796 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168796&action=review

> Source/WebCore/ChangeLog:15
> +        (WebCore):

Please omit lines like this even when the script generates them.

> Source/WebCore/rendering/RenderBox.cpp:1775
> +static bool stretchingVerticalFlexboxChild(const RenderObject* child)

This function name sounds like a function that returns the “stretching vertical flexbox child”. Probably best to add the word “is” or “should” to the name to disambiguate.

> Source/WebCore/rendering/RenderBox.cpp:1783
> +    if (parent->isFlexibleBox() && parent->style()->flexWrap() == FlexWrapNone && parent->style()->isColumnFlexDirection()
> +        && (child->style()->alignSelf() == AlignStretch || (child->style()->alignSelf() == AlignAuto && parent->style()->alignItems() == AlignStretch)))
> +        return true;

I think this could use a comment to explain why this is the correct set of checks. And maybe even some helper functions.

[Tony Chang]

Created attachment 168972 [details]
Patch

[Ojan Vafai]

Comment on attachment 168972 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168972&action=review

> Source/WebCore/rendering/RenderBox.cpp:1775
> +static bool flexboxChildHasStretchAlignment(const RenderObject* child)

Naming nit: how about flexItemHasStretchAlignment?

> Source/WebCore/rendering/RenderBox.cpp:1781
> +static bool isStretchingVerticalFlexboxChild(const RenderObject* child)

Naming nit: isStretchingColumnFlexItem

> Source/WebCore/rendering/RenderBox.cpp:1822
>          if (!parent()->style()->isColumnFlexDirection() || parent()->style()->flexWrap() != FlexWrapNone)

If you added isFlexboxRowOrMultiline(parent), then you could use it here and in isStretchingVerticalFlexboxChild. I think it would make the code a bit more readable.

[Tony Chang]

Created attachment 168984 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 168984 [details]
Patch for landing

Clearing flags on attachment: 168984

Committed r131481: <http://trac.webkit.org/changeset/131481>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.