Closed Bug 1512883 Opened 1 year ago Closed 1 year ago

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

Categories

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

64 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: Hallo89, Assigned: boris)

References

(Depends on 1 open bug)

Details

Attachments

(3 files)

Attached file 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
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?
Status: UNCONFIRMED → NEW
Component: CSS Transitions and Animations → DOM: Animation
Ever confirmed: true
Flags: needinfo?(boris.chiou)
Priority: -- → P1
Sure. I will take a look at it.
Assignee: nobody → boris.chiou
Flags: needinfo?(boris.chiou)
Status: NEW → ASSIGNED
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
(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>.
Depends on: 1514342
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.)
Add some tests to make sure we clamp the interpolated result with
negative easing function on circle(), ellipse(), and inset().

Depends on D14654
Pushed by boris.chiou@gmail.com:
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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14613 for changes under testing/web-platform/tests
https://hg.mozilla.org/mozilla-central/rev/70912a8087d7
https://hg.mozilla.org/mozilla-central/rev/c1b46f5fc73b
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.