border-box clip-paths jump around when outline changes

• Component: CSS
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

The reference box for clip paths is being computed incorrectly, resulting in the clip path changing when outlines change. This is breaking a test:

http://build.webkit.org/results/Apple%20Mavericks%20Debug%20WK1%20(Tests)/r164236%20(3035)/results.html

[Simon Fraser (smfr)]

In computeReferenceBox(), rootRelativeBounds() is probably the wrong box to use; it takes outlines into account.

[Simon Fraser (smfr)]

Created attachment 224414 [details]
Testcase; path changes on hover

[Simon Fraser (smfr)]

Created attachment 224418 [details]
Patch

[Dirk Schulze]

Comment on attachment 224418 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:3844
> +        LayoutRect referenceBox = computeReferenceBox(renderer(), clippingPath, offsetFromRoot, rootRelativeBounds);

I don't understand. If rootRelativeBounds takes outline into account (which makes sense, didn't think about that before) why does this not affect the dimension of rootRelativeBounds? Shouldn't the width and height of rootRelativeBounds have the outline as well?

I assume your test passes so it seems not the case. I would just like to know why. This doesn't seem logical.

[Dirk Schulze]

Comment on attachment 224418 [details]
Patch

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

> LayoutTests/css3/masking/clip-path-root-relative-bounds-expected.html:8
> +	-webkit-clip-path: circle(50%, 50%, 50%) border-box;

Could you use the new syntax please? This is the deprecated syntax that I would very much like to remove.

New syntax is circle(<radius> at <position>) and to archive the same as you did, just circle() :)

Anyway. Still don't understand why rootRelativeBounds takes the offset of outline but not the dimension.

[Simon Fraser (smfr)]

Comment on attachment 224418 [details]
Patch

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

>> Source/WebCore/rendering/RenderLayer.cpp:3844
>> +        LayoutRect referenceBox = computeReferenceBox(renderer(), clippingPath, offsetFromRoot, rootRelativeBounds);
> 
> I don't understand. If rootRelativeBounds takes outline into account (which makes sense, didn't think about that before) why does this not affect the dimension of rootRelativeBounds? Shouldn't the width and height of rootRelativeBounds have the outline as well?
> 
> I assume your test passes so it seems not the case. I would just like to know why. This doesn't seem logical.

rootRelativeBounds is just a bounding box for drawing, since it encompasses outlines and positioned descendants (which might be positioned to your top/left). So it's location is not the right thing to use to offset the border box.

The size of rootRelativeBounds IS affected by outline, but that only get used in the to-be-removed "BoxMissing" state.

[Simon Fraser (smfr)]

Created attachment 224569 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 224569 [details]
Patch

Dirk gave a review on webkit-dev

[Simon Fraser (smfr)]

https://trac.webkit.org/r164336

[Dirk Schulze]

(In reply to comment #8)
> (From update of attachment 224569 [details])
> Dirk gave a review on webkit-dev

Confirmative :)