1916646 - clip-path: rect does not floor values

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Philip Rogers]

Attachment: clip.html

Steps to reproduce:

https://drafts.csswg.org/css-shapes/#funcdef-basic-shape-rect uses syntax "top, right, bottom, and left" and states that "the second (right) and third (bottom) values are floored by the fourth (left) and second (top) values, respectively." Firefox does not implement this flooring.

For posterity, this was originally reported at https://crbug.com/364443021.

Actual results:

The attached testcase (clip.html) is from Example 7 in the spec, and should show no red (fully clipped red boxes).

Expected results:

The boxes are not fully clipped and red is visible.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Boris any chance you want to take this, or make it a good-first-bug and provide some context for someone else to take it?

Comment 2

[Boris Chiou [:boris]]

Looks like we also render clip-path:inset(10px 100% 100% 20px);, which is the same as clip-path:rect(10px 0 0 20px);, incorrectly.

Basically, the computed value of rect(10px 0 0 20px) is still inset(10px 100% 100% 20px), per spec, and we just need to tweak its used value when building its gfx path, i.e. update ComputeInsetRect() to handle the floor stuff in ShapeUtilis.cpp.

Comment 3

[Boris Chiou [:boris]]

Oh. I feel like the handling of floor values may be a spec issue.

We always convert the rect() into inset() at the computed time (https://drafts.csswg.org/css-shapes-1/#basic-shape-computed-values), and obviously we shouldn't do floor thing at the computed time.

Also, we should handle the case as an inset() function: we proportionally reduce the inset effect to 100% if the sum of the used dimension is larger than 100% (https://drafts.csswg.org/css-shapes-1/#funcdef-basic-shape-inset).

So it seems the floor value in rect() spec section is never used. We should file the spec issue to make it clear.

However, we still can fix this bug by using spec section of inset().

Comment 4

[Boris Chiou [:boris]]

I filed https://github.com/w3c/csswg-drafts/issues/10870.

Comment 5

[Boris Chiou [:boris]]

Attachment: Bug 1916646 - Floor right/bottom values of inset()/rect() to make sure that width and height of the rectangle are valid.

Per spec of rect(), the second (right) and third (bottom) values are
floored by the fourth (left) and second (top) values, respectively. This
is to avoid that they don't cross over the other edge. In our
implementation, we just clamp width and height to make sure they are not
negative. The both ways should be identical.

Note that the spec also mentions another way to handle the similiar case
in inset(), which would like to apply the algorithm mentioned in
"CSS Backgrounds 3 § 4.5 Overlapping Curves rules" to proportionally reduce
the inset effect to 100%. This may cause a different result from
flooring the values. I suspect this is a spec issue:
https://github.com/w3c/csswg-drafts/issues/10870.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c43f092dbbf
Floor right/bottom values of inset()/rect() to make sure that width and height of the rectangle are valid. r=dshin

Comment 7

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/3c43f092dbbf

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48273 for changes under testing/web-platform/tests

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot