Closed
Bug 1512883
Opened 5 years ago
Closed 5 years ago
Negative easing on a circle clip-path is able to crash the tab
Categories
(Core :: DOM: Animation, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla66
Tracking | Status | |
---|---|---|
firefox66 | --- | fixed |
People
(Reporter: Hallo89, Assigned: boris)
References
(Depends on 1 open bug)
Details
Attachments
(3 files)
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•5 years ago
|
||
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
Assignee | ||
Comment 2•5 years ago
|
||
Sure. I will take a look at it.
Assignee: nobody → boris.chiou
Flags: needinfo?(boris.chiou)
Assignee | ||
Updated•5 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•5 years ago
|
||
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
Assignee | ||
Comment 4•5 years ago
|
||
(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>.
Assignee | ||
Comment 5•5 years ago
|
||
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.)
Assignee | ||
Comment 6•5 years ago
|
||
Add some tests to make sure we clamp the interpolated result with negative easing function on circle(), ellipse(), and inset(). Depends on D14654
Assignee | ||
Comment 7•5 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba4c9b5d8197b28b0c02ce422f35078bd5f501ac
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
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/70912a8087d7 https://hg.mozilla.org/mozilla-central/rev/c1b46f5fc73b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox66:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Upstream PR merged
You need to log in
before you can comment on or make changes to this bug.
Description
•