Created attachment 8875567 [details] Test case The attached test case is a reduced test case from a failing test in dom/smil/test/test_smilCSSFromBy.xhtml The square should fade from opacity 1 to 0 because we have: <rect x="20" y="20" width="200" height="200" fill="purple"> <animate attributeName="opacity" from="1" by="-1" dur="3s" fill="freeze" /> </rect> However, it doesn't. When I debug it I see: Parsed opacity property value '1' => 1 Parsed opacity property value '-1' => 0 So it would appear that we are either failing to parse the negative value, or clamping it too early. As a result we effectively do: <animate attributeName="opacity" from="1" by="0" dur="3s" fill="freeze" />
I think we're doing the clamping stuff during the computing... http://searchfox.org/mozilla-central/rev/1a054419976437d0778a2b89be1b00207a744e94/servo/components/style/values/specified/mod.rs#707
I am leaving comments for someone who are going to fix this bug. What we need to do here is: 1) Add a flag that represents that current mode allows all numeric values for converting specified value to computed value 2) Set the flag true in Servo_GetAnimationValues (that is used only for SMIL) 3) Check the flag in Opacity.to_computed_value() and clamp the value only if the flag is false. Once we allow negative color component (bug 1364279), we also need to change Color.to_computed_value, I guess.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Thanks for the helpful instructions, Hiro. :-) https://treeherder.mozilla.org/#/jobs?repo=try&revision=88c2da6603021b38d166e1232149d51372991167
I didn't add a separate test case, but I checked that the sub-test in dom/smil/test/test_smilCSSFromBy.xhtml started passing.
Comment on attachment 8889331 [details] Bug 1371150 - Update test expectations. https://reviewboard.mozilla.org/r/160406/#review165666 Thanks for fixing this!
Attachment #8889331 - Flags: review?(hikezoe) → review+
Comment on attachment 8889332 [details] Bug 1371150 - style: Don't clamp Opacity values when computing them for SMIL. https://reviewboard.mozilla.org/r/160408/#review165660
Attachment #8889332 - Flags: review?(hikezoe) → review+
Attachment #8889332 - Attachment is obsolete: true
Comment on attachment 8889331 [details] Bug 1371150 - Update test expectations. https://reviewboard.mozilla.org/r/160406/#review165688
Attachment #8889331 - Flags: review?(cam) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/26d556a0f07c Update test expectations. r=hiro
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.