Bug 23658 – webkit should support box-sizing:padding-box

• Component: CSS
• Status: NEW

[Ojan Vafai]

While it's not in the spec, Gecko supports it and it's useful.

[Elliott Sprehn]

This is now in the spec: http://www.w3.org/TR/css3-ui/#box-sizing0

[Elliott Sprehn]

Looks like this is just changes in:

RenderBox::computeBorderBoxLogical(Width|Height) and computeContentBoxLogical(Width|Height).

[Elliott Sprehn]

Created attachment 156798 [details]
Patch

Attempt at implementing padding box. This doesn't work though and I don't know why. getComputedStyle(e).boxSizing always returns content-box even though it goes through the isValidKeywordPropertyAndValue check and I changed CSSComputedStyleDeclaration. Anyone know what I missed?

[WebKit Review Bot]

Comment on attachment 156798 [details]
Patch

Attachment 156798 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13444629

New failing tests:
fast/css/resize-single-axis.html
fast/css/resize-corner-tracking-transformed.html
fast/css/resize-corner-tracking.html

[WebKit Review Bot]

Created attachment 156821 [details]
Archive of layout-test-results from gce-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[WebKit Review Bot]

Comment on attachment 156798 [details]
Patch

Attachment 156798 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13446545

New failing tests:
fast/css/resize-single-axis.html
fast/css/resize-corner-tracking-transformed.html
fast/css/resize-corner-tracking.html

[WebKit Review Bot]

Created attachment 156837 [details]
Archive of layout-test-results from gce-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Elliott Sprehn]

(In reply to comment #3)
> Created an attachment (id=156798) [details]
> Patch
> 
> Attempt at implementing padding box. This doesn't work though and I don't know why. getComputedStyle(e).boxSizing always returns content-box even though it goes through the isValidKeywordPropertyAndValue check and I changed CSSComputedStyleDeclaration. Anyone know what I missed?

I figured this out. In StyleBoxData m_boxSizing is declared a 1 bit field, but EBoxSizing now has 3 values so when you assigned the value 2, it gave you the value of 1 back from the field resulting in CONTENT_BOX.

[Elliott Sprehn]

Created attachment 157618 [details]
Patch

Fixed implementation of padding-box. This still needs a bunch of tests. My local test isnt suitable as a LayoutTest.

[Ojan Vafai]

Comment on attachment 157618 [details]
Patch

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

R- for lack of tests and for the RenderLayer/RenderReplaced issues. Otherwise, patch looks good.

I'd prefer to fix the table cases in this patch and just get it over with. The hard part there is just writing tests to verify it's doing the right thing. But if you'd prefer to do the table/table-cell cases in a follow-up patch, that's fine too.

Looks like there's another use in RenderTable and one in RenderTableCell that you missed. If you don't want to fix them in this patch, please add FIXMEs there as well.

> Source/WebCore/rendering/RenderLayer.cpp:1967
> +            baseWidth -= renderer->borderLogicalWidth();

This is setting the physical width below, so, this should just use the physical border width.

> Source/WebCore/rendering/RenderLayer.cpp:1982
> +            baseHeight -= renderer->borderLogicalHeight();

Ditto, but for height obviously.

> Source/WebCore/rendering/RenderReplaced.cpp:452
> +            maxWidth = maxWidth + borderLeft() + borderRight();

This should just be borderLogicalWidth. The borderAndPaddingWidth call above is wrong. It should be borderAndPaddingLogicalWidth.

> Source/WebCore/rendering/RenderTable.cpp:278
> +        // FIXME: We might need to handle PADDING_BOX here.

I'm not too familiar with this code, but I think we do need to. Just need to use borderLogicalWidth I think. Did you have trouble making a testcase?

> Source/WebCore/rendering/style/StyleBoxData.h:84
> +    unsigned m_boxSizing : 2; // EBoxSizing

Sigh. Sorry I didn't catch this.

[Brent Fulgham]

It doesn't seem like WebKit supports this in Safari 15.

[Radar WebKit Bug Importer]

<rdar://problem/96976634>

[Ahmad Saleem]

(In reply to Brent Fulgham from comment #11)
> It doesn't seem like WebKit supports this in Safari 15.

As per spec - if I am not wrong, we only have:

Value:	content-box | border-box

https://w3c.github.io/csswg-drafts/css-sizing/#box-sizing

Plus as per MDN Compatibility table, 'padding-box' support is missing from all browsers.

https://developer.mozilla.org/en-US/docs/Web/CSS/box-sizing#browser_compatibility

Comment 01 mentions that it was in Firefox but MDN table suggests that it was removed in Firefox 50 version.

Do we have any spec discussion? I tried to find using 'padding-box' text on here:

https://github.com/w3c/csswg-drafts/issues

but couldn't find much. Appreciate if you can help me for my learning as well.