1512883 - Negative easing on a circle clip-path is able to crash the tab

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect, P1)

Description

[Malus]

Attachment: clip-path crash.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

I took an element equipped with a clip-path with the value of `circle(<any value>)` and a hover effect which makes the circle value bigger or smaller. Then I went crazy and put a transition with a big negative value on its easing (something like `cubic-bezier(0,0,1,-20)`, how big it must be depends on the circle's size).


Actual results:

Triggering the hover effect, the circle() value transitioned to a negative value, an unexpected behavior, making the tab crash.
See the attachment for a demonstration


Expected results:

Gecko should not have tried to re-render the clip-path with a negative value, not making the tab crash.
I am pretty certain that this bug can also be applied to other clip-path values (other than circle) and maybe other CSS properties as well

Comment 1

[Brian Birtles (:birtles)]

Thanks for this report. Moving to DOM: Animation since this is likely to be a bug in the interpolation of clip path values in general and not limited to CSS animations/transitions.

Boris, I think you are most familiar with the path interpolation code. Mind having a look?

Comment 2

[Boris Chiou [:boris]]

Sure. I will take a look at it.

Comment 3

[Boris Chiou [:boris]]

For circle() and ellipse(), we don't make sure that the interpolated result is always non-negative, as what we did for other non-negative properties [1]. Therefore, I think we may implement ToAnimatedValue for ShapeSource<...>, and use AnimatedValue, instead of ComputedValue, for interpolation.

[1] https://github.com/servo/servo/pull/17783

Comment 4

[Boris Chiou [:boris]]

(In reply to Boris Chiou [:boris] from comment #3)
> For circle() and ellipse(), we don't make sure that the interpolated result
> is always non-negative, as what we did for other non-negative properties
> [1]. Therefore, I think we may implement ToAnimatedValue for
> ShapeSource<...>, and use AnimatedValue, instead of ComputedValue, for
> interpolation.
> 
> [1] https://github.com/servo/servo/pull/17783

And inset(), which uses <border-radius>.

Comment 5

[Boris Chiou [:boris]]

Attachment: Bug 1512883 - Part 1: Clamp to non-negative value after doing interpolation for circle(), ellipse(), and inset().

Implement ToAnimatedValue for ShapeSource and clamp the interpolated
results, the radius of circle()/ellipse() and the border-radius of inset(),
into non-negative values. (Note: we may get negative values when using
negative easing function, so the clamp is necessary to avoid the incorrect
result or crash.)

Comment 6

[Boris Chiou [:boris]]

Attachment: Bug 1512883 - Part 2: Tests.

Add some tests to make sure we clamp the interpolated result with
negative easing function on circle(), ellipse(), and inset().

Depends on D14654

Comment 7

[Boris Chiou [:boris]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba4c9b5d8197b28b0c02ce422f35078bd5f501ac

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70912a8087d7
Part 1: Clamp to non-negative value after doing interpolation for circle(), ellipse(), and inset(). r=emilio,birtles
https://hg.mozilla.org/integration/autoland/rev/c1b46f5fc73b
Part 2: Tests. r=birtles

Comment 9

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

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

Comment 10

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/70912a8087d7
https://hg.mozilla.org/mozilla-central/rev/c1b46f5fc73b

Comment 11

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

Upstream PR merged