Bug 88741 – Width not properly recalculated with -webkit-box-sizing: border-box

• Component: CSS
• Status: RESOLVED CONFIGURATION CHANGED

[Trevor Burnham]

When an element in border-box mode with fixed width has a change in padding-left (e.g. due to a hover state), its child will not expand to fill the space.  Here's a test case: http://jsfiddle.net/TrevorBurnham/gCAsj/ (Note that in Firefox 11, this example works as expected, complete with smooth transition.) The same bug occurs with padding-right: http://jsfiddle.net/TrevorBurnham/fh2qD/ (Again, this works fine in Firefox.) Changes in padding-top and padding-bottom appear to work properly.

[Kulanthaivel Palanichamy]

Created attachment 148879 [details]
Proposed patch

Ideally we should have been able to detect content box size change in RenderBlock::recomputeLogicalWidth(), but 
unfortunately we never keep the old style in RenderObject to compare old border and padding values.
One another possible fix could be compare the old and new styles in RenderBox::styleDidChange() for
border and padding change when box-sizing is border-box, set setChildNeedsLayout(true),
then force relayout on children in RenderBlock::layoutBlockChildren() if normalChildNeedsLayout() is true.

[Kulanthaivel Palanichamy]

Created attachment 149616 [details]
Proposed patch

Not convinced by my previous patch.

[Julien Chaffraix]

Comment on attachment 149616 [details]
Proposed patch

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

> LayoutTests/fast/box-sizing/box-sizing-dynamic-padding-with-relative-width-child.html:18
> +        setTimeout('test2();', 100);

I don't understand the need for your 100ms delays. They shouldn't be needed to get proper results after a layout.

Moreover, they make the test take at least 500ms which is a very long time for a test case.

> Source/WebCore/rendering/RenderBlock.cpp:1381
> +    return oldWidth != logicalWidth() || oldColumnWidth != desiredColumnWidth() || contentBoxDirty();

I feel like the real fix should be here: the logic should be able to handle box-sizing change properly. There will be several ways of handling that (in order of preference):
* introduce a wrapper around LayoutUnit that is aware of the length's type (border vs content) we are manipulating to avoid mistakes (it will require some work but is a good long term change as we could catch dumb mistakes)
* standardize on having logicalWidth / logicalHeight always be the border-box / content-box. In this case, rename logicalWidth to match this decision.
* invalidate our width in styleDidChange to make sure we return 'true' here.

> Source/WebCore/rendering/RenderObject.h:1003
> +        ADD_BOOLEAN_BITFIELD(contentBoxDirty, ContentBoxDirty);

As discussed, I really feel this is a pretty weak use of our last remaining bit. Changing box-sizing is fairly uncommon and I would rather keep this bit for something more useful.

[Brent Fulgham]

Safari, Chrome, and Firefox all agree on rendering for this test case. I don't believe there is any remaining compatibility issue.