If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Assertion failure: aDilutionRatio >= 0.0 && aDilutionRatio <= 1.0 (Dilution ratio should be in [0, 1]), at StyleAnimationValue.cpp:1980

RESOLVED FIXED in Firefox 53

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
9 months ago
8 months ago

People

(Reporter: Tomcat, Assigned: hiro)

Tracking

(Blocks: 1 bug, {assertion})

unspecified
mozilla53
assertion
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 wontfix, firefox52 wontfix, firefox53+ fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

9 months ago
Created attachment 8823551 [details]
bughunter stack from aurora build

Found via bughunter and reproduced on latest windows 7 debug tinderbox trunk build.

Steps to reproduce

-> Load http://divinityhtml.oxygenna.com/genesis/index.html
--> Assertion failure on load

affects trunk to beta according to bughunter data
(Reporter)

Comment 1

9 months ago
[Tracking Requested - why for this release]:

affects debug builds only, a opt build didn't crashed
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox53: --- → affected
tracking-firefox53: --- → ?
searchfox link:
http://searchfox.org/mozilla-central/rev/22be34bcc4d5c56b62482a537bba77a6cdce117b/layout/style/StyleAnimationValue.cpp#1979-1980

This is an assertion added in bug 1294614. hiro, mind taking a look?
Blocks: 1294614
Sure.  An amimation-timing-function for box-shadow animation, that is acually cubic-bezier(0.74, 0.01, 0.35, 1.01), produces values out of range [0, 1].  We don't need to assert there.
Assignee: nobody → hiikezoe
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 5

9 months ago
mozreview-review
Comment on attachment 8823855 [details]
Bug 1328535 - Remove range-checking assertion for aDilutionRatio, in color animation code.

https://reviewboard.mozilla.org/r/102336/#review102732

r=me with nits addressed.

Two commit message nits:
> Bug 1328535 - Don't assert even if |aDillutionRatio| is out of range of [0, 1]. r?dholbert

1) Typo: you've got an extra "l" in aDillutionRatio there.
2) This doesn't have enough context to suggest what part of the code it's touching. You should mention animation and/or colors, to give a bit more context.

Suggested replacement message:
Bug 1328535 - Remove range-checking assertion for aDilutionRatio, in color animation code. r?dholbert

::: layout/style/test/test_transitions_per_property.html:1672
(Diff revision 1)
>         "shadow-valued property " + prop + " (radius): clamping of negatives");
>      isnot(vals[6], "0px",
>            "shadow-valued property " + prop + " (spread): clamping of negatives");
>    }
> +
> +  // A test case that timing functin produces values greater than 1.0.

typo: s/functin/function/

::: layout/style/test/test_transitions_per_property.html:1688
(Diff revision 1)
> +  // converted to 0.65.
> +  is(cs.getPropertyValue(prop), "rgba(100, 100, 100, 0.65) 0px 0px 0px" + spreadStr,
> +     "shadow-valued property " + prop + ": interpolation of shadows with " +
> +     "timing function which produces values greater than 1.0");
> +
>    div.style.setProperty("transition-timing-function", "linear", "");

Here I think you're intending to restore the original value (which we set elsewhere in this file) -- right?

This seems a little fragile. It'd probably be better to capture the value that you're stomping on (before you stomp on it), and then restore it at the end. Something like this:
====
  var origTimingFunc = div.style.getPropertyValue("transition-timing-function");

 [... your test code here ...]

 // Clean up:
 div.style.setProperty("transition-timing-function", origTimingFunc);
====

How does that sound?
Attachment #8823855 - Flags: review?(dholbert) → review+
(Assignee)

Comment 6

9 months ago
mozreview-review-reply
Comment on attachment 8823855 [details]
Bug 1328535 - Remove range-checking assertion for aDilutionRatio, in color animation code.

https://reviewboard.mozilla.org/r/102336/#review102732

> Here I think you're intending to restore the original value (which we set elsewhere in this file) -- right?
> 
> This seems a little fragile. It'd probably be better to capture the value that you're stomping on (before you stomp on it), and then restore it at the end. Something like this:
> ====
>   var origTimingFunc = div.style.getPropertyValue("transition-timing-function");
> 
>  [... your test code here ...]
> 
>  // Clean up:
>  div.style.setProperty("transition-timing-function", origTimingFunc);
> ====
> 
> How does that sound?

Indeed, it quite makes sense. Thanks!
Comment hidden (mozreview-request)

Comment 8

9 months ago
Pushed by hiikezoe@mozilla-japan.org:
https://hg.mozilla.org/integration/autoland/rev/e4be17b09b8d
Remove range-checking assertion for aDilutionRatio, in color animation code. r=dholbert

Comment 9

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e4be17b09b8d
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
tracking-firefox53: ? → +
Too late for 51. Mark 51 won't fix.
status-firefox51: affected → wontfix
We might as well wontfix for 52, too. The patch here just removes a bogus assertion -- the only impact is on debug builds. So there's no compelling reason to uplift.

 --> updating 52 status to make that official.
status-firefox52: affected → wontfix
You need to log in before you can comment on or make changes to this bug.