Bug 1390367 - Pass SMIL mochitests of stroke-* even if a computed value has no px unit. r=birtles
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Tue, 05 Sep 2017 13:15:32 +0900
changeset 378831 09acc539e06593f437465b2ddf1eddd3be15baea
parent 378830 0a61f65ac5edacc19f8d8743a25b33e4249f939e
child 378832 00f4b42eb881e1ae5ad9465b48f666a73045ebdf
push id32442
push userarchaeopteryx@coole-files.de
push dateTue, 05 Sep 2017 09:39:11 +0000
treeherdermozilla-central@35bd47b6e5ac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles
bugs1390367, 1379908
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1390367 - Pass SMIL mochitests of stroke-* even if a computed value has no px unit. r=birtles Currently, Gecko converts lengths in stroke-* properties to numbers when creating animation values and hence the result of animation is always a unitless value for these properties. Servo, on the other hand, converts lengths for these properties to numbers during interpolation and hence sometimes the result of animation is a unitless value, and other times (such as when skipping interpolation) it is not. Other browsers produce lengths with px units and ultimately we should align with that behavior. As a result, this patch updates various SMIL mochitests to: * Expect animation values with px unit * Compare values by stripping px units as a temporary measure until we correctly serialize these values with px (bug 1379908). MozReview-Commit-ID: IsT26DKkgiP
dom/smil/test/db_smilCSSFromBy.js
dom/smil/test/db_smilCSSFromTo.js
dom/smil/test/db_smilCSSPaced.js
dom/smil/test/mochitest.ini
dom/smil/test/smilTestUtils.js
dom/smil/test/test_smilTextZoom.xhtml
--- a/dom/smil/test/db_smilCSSFromBy.js
+++ b/dom/smil/test/db_smilCSSFromBy.js
@@ -55,37 +55,23 @@ var _fromByTestLists =
   lengthNoUnits: [
     new AnimTestcaseFromBy("0", "50",  { fromComp: "0px", // 0 acts like 0px
                                          midComp:  "25px",
                                          toComp:   "50px"}),
     new AnimTestcaseFromBy("30", "10", { fromComp: "30px",
                                          midComp:  "35px",
                                          toComp:   "40px"}),
   ],
-  lengthNoUnitsSVG: [
-    new AnimTestcaseFromBy("0", "50",  { fromComp: "0",
-                                         midComp:  "25",
-                                         toComp:   "50"}),
-    new AnimTestcaseFromBy("30", "10", { fromComp: "30",
-                                         midComp:  "35",
-                                         toComp:   "40"}),
-  ],
   lengthPx: [
     new AnimTestcaseFromBy("0px", "8px", { fromComp: "0px",
                                            midComp: "4px",
                                            toComp: "8px"}),
-    new AnimTestcaseFromBy("1px", "10px", { midComp: "6px", toComp: "11px"}),
-  ],
-  lengthPxSVG: [
-    new AnimTestcaseFromBy("0px", "8px", { fromComp: "0",
-                                           midComp: "4",
-                                           toComp: "8"}),
-    new AnimTestcaseFromBy("1px", "10px", { fromComp: "1",
-                                            midComp: "6",
-                                            toComp: "11"}),
+    new AnimTestcaseFromBy("1px", "10px", { fromComp: "1px",
+                                            midComp: "6px",
+                                            toComp: "11px"}),
   ],
   opacity: [
     new AnimTestcaseFromBy("1", "-1", { midComp: "0.5", toComp: "0"}),
     new AnimTestcaseFromBy("0.4", "-0.6", { midComp: "0.1", toComp: "0"}),
     new AnimTestcaseFromBy("0.8", "-1.4", { midComp: "0.1", toComp: "0"},
                            "opacities with abs val >1 get clamped too early"),
     new AnimTestcaseFromBy("1.2", "-0.6", { midComp: "0.9", toComp: "0.6"},
                            "opacities with abs val >1 get clamped too early"),
@@ -171,11 +157,11 @@ var gFromByBundles =
   new TestcaseBundle(gPropList.stroke_dasharray, [
     // These testcases implicitly have no effect, because stroke-dasharray is
     // non-additive (and is declared as such in db_smilCSSPropertyList.js)
     new AnimTestcaseFromBy("none", "5"),
     new AnimTestcaseFromBy("10", "5"),
     new AnimTestcaseFromBy("1", "2, 3"),
   ]),
   new TestcaseBundle(gPropList.stroke_width,
-                     [].concat(_fromByTestLists.lengthNoUnitsSVG,
-                               _fromByTestLists.lengthPxSVG))
+                     [].concat(_fromByTestLists.lengthNoUnits,
+                               _fromByTestLists.lengthPx))
 ];
--- a/dom/smil/test/db_smilCSSFromTo.js
+++ b/dom/smil/test/db_smilCSSFromTo.js
@@ -83,64 +83,45 @@ var _fromToTestLists = {
                                          toComp:   "20px"}),
     new AnimTestcaseFromTo("50",  "0", { fromComp: "50px",
                                          midComp:  "25px",
                                          toComp:    "0px"}),
     new AnimTestcaseFromTo("30", "80", { fromComp: "30px",
                                          midComp:  "55px",
                                          toComp:   "80px"}),
   ],
-  lengthNoUnitsSVG: [
-    new AnimTestcaseFromTo("0",  "20", { fromComp:  "0",
-                                         midComp:  "10",
-                                         toComp:   "20"}),
-    new AnimTestcaseFromTo("50",  "0", { fromComp: "50",
-                                         midComp:  "25",
-                                         toComp:    "0"}),
-    new AnimTestcaseFromTo("30", "80", { fromComp: "30",
-                                         midComp:  "55",
-                                         toComp:   "80"}),
-  ],
   lengthPx: [
     new AnimTestcaseFromTo("0px", "12px", { fromComp: "0px",
-                                            midComp:  "6px"}),
-    new AnimTestcaseFromTo("16px", "0px", { midComp: "8px",
-                                            toComp:  "0px"}),
-    new AnimTestcaseFromTo("10px", "20px", { midComp: "15px"}),
-    new AnimTestcaseFromTo("41px", "1px", { midComp: "21px"}),
-  ],
-  lengthPxSVG: [
-    new AnimTestcaseFromTo("0px", "12px", { fromComp: "0",
-                                            midComp:  "6",
-                                            toComp:  "12"}),
-    new AnimTestcaseFromTo("16px", "0px", { fromComp: "16",
-                                            midComp:   "8",
-                                            toComp:    "0"}),
-    new AnimTestcaseFromTo("10px", "20px", { fromComp: "10",
-                                             midComp:  "15",
-                                             toComp:   "20"}),
-    new AnimTestcaseFromTo("41px", "1px", { fromComp: "41",
-                                            midComp:  "21",
-                                            toComp:    "1"}),
+                                            midComp:  "6px",
+                                            toComp:   "12px"}),
+    new AnimTestcaseFromTo("16px", "0px", { fromComp: "16px",
+                                            midComp:  "8px",
+                                            toComp:   "0px"}),
+    new AnimTestcaseFromTo("10px", "20px", { fromComp: "10px",
+                                             midComp:  "15px",
+                                             toComp:   "20px"}),
+    new AnimTestcaseFromTo("41px", "1px", { fromComp: "41px",
+                                            midComp:  "21px",
+                                            toComp:   "1px"}),
   ],
   lengthPctSVG: [
     new AnimTestcaseFromTo("20.5%", "0.5%", { midComp: "10.5%" }),
   ],
   lengthPxPctSVG: [
     new AnimTestcaseFromTo("10px", "10%", { midComp: "15px"},
                            "need support for interpolating between " +
                            "px and percent values"),
   ],
   lengthPxNoUnitsSVG: [
-    new AnimTestcaseFromTo("10", "20px", { fromComp: "10",
-                                           midComp:  "15",
-                                           toComp:   "20"}),
-    new AnimTestcaseFromTo("10px", "20", { fromComp: "10",
-                                           midComp:  "15",
-                                           toComp:   "20"}),
+    new AnimTestcaseFromTo("10", "20px", { fromComp: "10px",
+                                           midComp:  "15px",
+                                           toComp:   "20px"}),
+    new AnimTestcaseFromTo("10px", "20", { fromComp: "10px",
+                                           midComp:  "15px",
+                                           toComp:   "20px"}),
   ],
   opacity: [
     new AnimTestcaseFromTo("1", "0", { midComp: "0.5" }),
     new AnimTestcaseFromTo("0.2", "0.12", { midComp: "0.16" }),
     new AnimTestcaseFromTo("0.5", "0.7", { midComp: "0.6" }),
     new AnimTestcaseFromTo("0.5", "inherit",
                            { midComp: "0.75", toComp: "1" }),
     // Make sure we don't clamp out-of-range values before interpolation
@@ -405,18 +386,18 @@ var gFromToBundles = [
     new AnimTestcaseFromTo("1", "2, 3", { fromComp: "1, 1",
                                           midComp: "1.5, 2"}),
     new AnimTestcaseFromTo("2, 8", "6", { midComp: "4, 7"}),
     new AnimTestcaseFromTo("1, 3", "1, 3, 5, 7, 9",
                            { fromComp: "1, 3, 1, 3, 1, 3, 1, 3, 1, 3",
                              midComp:  "1, 3, 3, 5, 5, 2, 2, 4, 4, 6"}),
   ])),
   new TestcaseBundle(gPropList.stroke_dashoffset,
-                     [].concat(_fromToTestLists.lengthNoUnitsSVG,
-                               _fromToTestLists.lengthPxSVG,
+                     [].concat(_fromToTestLists.lengthNoUnits,
+                               _fromToTestLists.lengthPx,
                                _fromToTestLists.lengthPxPctSVG,
                                _fromToTestLists.lengthPctSVG,
                                _fromToTestLists.lengthPxNoUnitsSVG)),
   new TestcaseBundle(gPropList.stroke_linecap, [
     new AnimTestcaseFromTo("butt", "round"),
     new AnimTestcaseFromTo("round", "square"),
   ]),
   new TestcaseBundle(gPropList.stroke_linejoin, [
@@ -424,23 +405,23 @@ var gFromToBundles = [
     new AnimTestcaseFromTo("round", "bevel"),
   ]),
   new TestcaseBundle(gPropList.stroke_miterlimit, [
     new AnimTestcaseFromTo("1", "2", { midComp: "1.5" }),
     new AnimTestcaseFromTo("20.1", "10.1", { midComp: "15.1" }),
   ]),
   new TestcaseBundle(gPropList.stroke_opacity, _fromToTestLists.opacity),
   new TestcaseBundle(gPropList.stroke_width,
-                     [].concat(_fromToTestLists.lengthNoUnitsSVG,
-                               _fromToTestLists.lengthPxSVG,
+                     [].concat(_fromToTestLists.lengthNoUnits,
+                               _fromToTestLists.lengthPx,
                                _fromToTestLists.lengthPxPctSVG,
                                _fromToTestLists.lengthPctSVG,
                                _fromToTestLists.lengthPxNoUnitsSVG, [
     new AnimTestcaseFromTo("inherit", "7px",
-                           { fromComp: "1", midComp: "4", toComp: "7" }),
+                           { fromComp: "1px", midComp: "4px", toComp: "7px" }),
   ])),
   new TestcaseBundle(gPropList.text_anchor, [
     new AnimTestcaseFromTo("start", "middle"),
     new AnimTestcaseFromTo("middle", "end"),
   ]),
   new TestcaseBundle(gPropList.text_decoration, [
     new AnimTestcaseFromTo("none", "underline"),
     new AnimTestcaseFromTo("overline", "line-through"),
--- a/dom/smil/test/db_smilCSSPaced.js
+++ b/dom/smil/test/db_smilCSSPaced.js
@@ -82,64 +82,32 @@ var _pacedTestLists =
     new AnimTestcasePaced("10; 12; 8",
                           { comp0:   "10px",
                             comp1_6: "11px",
                             comp1_3: "12px",
                             comp2_3: "10px",
                             comp1:    "8px"
                           }),
   ],
-  lengthNoUnitsSVG : [
-    new AnimTestcasePaced("2; 0; 4",
-                          { comp0:   "2",
-                            comp1_6: "1",
-                            comp1_3: "0",
-                            comp2_3: "2",
-                            comp1:   "4"
-                          }),
-    new AnimTestcasePaced("10; 12; 8",
-                          { comp0:   "10",
-                            comp1_6: "11",
-                            comp1_3: "12",
-                            comp2_3: "10",
-                            comp1:   "8"
-                          }),
-  ],
   lengthPx : [
     new AnimTestcasePaced("0px; 2px; 6px",
                           { comp0:   "0px",
                             comp1_6: "1px",
                             comp1_3: "2px",
                             comp2_3: "4px",
                             comp1:   "6px"
                           }),
     new AnimTestcasePaced("10px; 12px; 8px",
                           { comp0:   "10px",
                             comp1_6: "11px",
                             comp1_3: "12px",
                             comp2_3: "10px",
                             comp1:   "8px"
                           }),
   ],
-  lengthPxSVG : [
-    new AnimTestcasePaced("0px; 2px; 6px",
-                          { comp0:   "0",
-                            comp1_6: "1",
-                            comp1_3: "2",
-                            comp2_3: "4",
-                            comp1:   "6"
-                          }),
-    new AnimTestcasePaced("10px; 12px; 8px",
-                          { comp0:   "10",
-                            comp1_6: "11",
-                            comp1_3: "12",
-                            comp2_3: "10",
-                            comp1:   "8"
-                          }),
-  ],
   lengthPctSVG : [
     new AnimTestcasePaced("5%; 6%; 4%",
                           { comp0:   "5%",
                             comp1_6: "5.5%",
                             comp1_3: "6%",
                             comp2_3: "5%",
                             comp1:   "4%"
                           }),
@@ -303,19 +271,19 @@ var gPacedBundles =
                           { comp0:   "7, 7, 7",
                             comp1_6: "7, 8.5, 5",
                             comp1_3: "7, 10, 3",
                             comp2_3: "4, 6, 3",
                             comp1:   "1, 2, 3"
                           }),
   ])),
   new TestcaseBundle(gPropList.stroke_dashoffset,
-                     [].concat(_pacedTestLists.lengthNoUnitsSVG,
-                               _pacedTestLists.lengthPxSVG,
+                     [].concat(_pacedTestLists.lengthNoUnits,
+                               _pacedTestLists.lengthPx,
                                _pacedTestLists.lengthPctSVG,
                                _pacedTestLists.lengthPxPctSVG)),
   new TestcaseBundle(gPropList.stroke_width,
-                     [].concat(_pacedTestLists.lengthNoUnitsSVG,
-                               _pacedTestLists.lengthPxSVG,
+                     [].concat(_pacedTestLists.lengthNoUnits,
+                               _pacedTestLists.lengthPx,
                                _pacedTestLists.lengthPctSVG,
                                _pacedTestLists.lengthPxPctSVG)),
   // XXXdholbert TODO: test 'stroke-dasharray' once we support animating it
 ];
--- a/dom/smil/test/mochitest.ini
+++ b/dom/smil/test/mochitest.ini
@@ -14,58 +14,51 @@ support-files =
 
 [test_smilAccessKey.xhtml]
 [test_smilAnimateMotion.xhtml]
 [test_smilAnimateMotionInvalidValues.xhtml]
 [test_smilAnimateMotionOverrideRules.xhtml]
 [test_smilBackwardsSeeking.xhtml]
 [test_smilCSSFontStretchRelative.xhtml]
 [test_smilCSSFromBy.xhtml]
-skip-if = stylo
 [test_smilCSSFromTo.xhtml]
-skip-if = stylo
 # [test_smilCSSInherit.xhtml]
 # disabled until bug 501183 is fixed
 [test_smilCSSInvalidValues.xhtml]
 [test_smilCSSPaced.xhtml]
-skip-if = stylo
 [test_smilChangeAfterFrozen.xhtml]
-skip-if = stylo
+skip-if = stylo # bug 1358955.
 [test_smilConditionalProcessing.html]
 [test_smilContainerBinding.xhtml]
 [test_smilCrossContainer.xhtml]
 [test_smilDynamicDelayedBeginElement.xhtml]
 [test_smilExtDoc.xhtml]
 skip-if = toolkit == 'android'
 [test_smilFillMode.xhtml]
 [test_smilGetSimpleDuration.xhtml]
 [test_smilGetStartTime.xhtml]
 [test_smilHyperlinking.xhtml]
 [test_smilInvalidValues.html]
 [test_smilKeySplines.xhtml]
 [test_smilKeyTimes.xhtml]
 [test_smilKeyTimesPacedMode.xhtml]
 [test_smilMappedAttrFromBy.xhtml]
-skip-if = stylo
 [test_smilMappedAttrFromTo.xhtml]
-skip-if = stylo
 [test_smilMappedAttrPaced.xhtml]
-skip-if = stylo
 [test_smilMinTiming.html]
 [test_smilRepeatDuration.html]
 [test_smilRepeatTiming.xhtml]
 skip-if = toolkit == 'android' #TIMED_OUT
 [test_smilReset.xhtml]
 [test_smilRestart.xhtml]
 [test_smilSetCurrentTime.xhtml]
 [test_smilSync.xhtml]
 [test_smilSyncTransform.xhtml]
 [test_smilSyncbaseTarget.xhtml]
 [test_smilTextZoom.xhtml]
-skip-if = stylo
 [test_smilTimeEvents.xhtml]
 [test_smilTiming.xhtml]
 [test_smilTimingZeroIntervals.xhtml]
 [test_smilUpdatedInterval.xhtml]
 [test_smilValues.xhtml]
 [test_smilWithTransition.html]
 [test_smilWithXlink.xhtml]
 [test_smilXHR.xhtml]
--- a/dom/smil/test/smilTestUtils.js
+++ b/dom/smil/test/smilTestUtils.js
@@ -108,16 +108,19 @@ var SMILUtil =
       computedStyle = SMILUtil.getComputedStyleSimple(elem, propName);
     }
     return computedStyle;
   },
 
   getMotionFakeAttributeName : function() {
     return "_motion";
   },
+
+  // Return stripped px value from specified value.
+  stripPx: str => str.replace(/px\s*$/, ''),
 };
 
 
 var CTMUtil =
 {
   CTM_COMPONENTS_ALL    : ["a", "b", "c", "d", "e", "f"],
   CTM_COMPONENTS_ROTATE : ["a", "b", "c", "d" ],
 
@@ -399,16 +402,29 @@ AnimTestcase.prototype =
   },
 
   seekAndTest : function(aSeekList, aTargetElem, aTargetAttr)
   {
     var svg = document.getElementById("svg");
     for (var i in aSeekList) {
       var entry = aSeekList[i];
       SMILUtil.getSVGRoot().setCurrentTime(entry[0]);
+
+      // Bug 1379908: The computed value of stroke-* properties should be
+      // serialized with px units, but currently Gecko and Servo don't do that
+      // when animating these values.
+      if (['stroke-width',
+           'stroke-dasharray',
+           'stroke-dashoffset'].includes(aTargetAttr.attrName)) {
+        var attr = SMILUtil.stripPx(
+          SMILUtil.getAttributeValue(aTargetElem, aTargetAttr));
+        var expectedVal = SMILUtil.stripPx(entry[1]);
+        is(attr, expectedVal, entry[2]);
+        return;
+      }
       is(SMILUtil.getAttributeValue(aTargetElem, aTargetAttr),
          entry[1], entry[2]);
     }
   },
 
   // methods that expect to be overridden in subclasses
   buildSeekListStatic : function(aAnimAttr, aBaseVal,
                                  aTimeData, aReasonStatic) {},
--- a/dom/smil/test/test_smilTextZoom.xhtml
+++ b/dom/smil/test/test_smilTextZoom.xhtml
@@ -25,16 +25,26 @@
 <script class="testbody" type="text/javascript">
 <![CDATA[
 SimpleTest.waitForExplicitFinish();
 
 // Helper function
 function verifyStyle(aNode, aPropertyName, aExpectedVal)
 {
   var computedVal = SMILUtil.getComputedStyleSimple(aNode, aPropertyName);
+
+  // Bug 1379908: The computed value of stroke-* properties should be
+  // serialized with px units, but currently Gecko and Servo don't do that
+  // when animating these values.
+  if ('stroke-width' == aPropertyName) {
+    var expectedVal = SMILUtil.stripPx(aExpectedVal);
+    var unitlessComputedVal = SMILUtil.stripPx(computedVal);
+    is(unitlessComputedVal, expectedVal, "computed value of " + aPropertyName);
+    return;
+  }
   is(computedVal, aExpectedVal, "computed value of " + aPropertyName);
 }
 
 function main()
 {
   // Start out pause
   var svg = SMILUtil.getSVGRoot();
   ok(svg.animationsPaused(), "should be paused by <svg> load handler");
@@ -53,26 +63,26 @@ function main()
     // property, which should _not_ be affected by textZoom.
     var text = document.getElementsByTagName("text")[0];
     var rect = document.getElementsByTagName("rect")[0];
 
     verifyStyle(text, "font-size",    "5px");
     verifyStyle(rect, "stroke-width", "5px");
     svg.setCurrentTime(1);
     verifyStyle(text, "font-size",    "20px");
-    verifyStyle(rect, "stroke-width", "20");
+    verifyStyle(rect, "stroke-width", "20px");
     svg.setCurrentTime(1.5);
     verifyStyle(text, "font-size",    "30px");
-    verifyStyle(rect, "stroke-width", "30");
+    verifyStyle(rect, "stroke-width", "30px");
     svg.setCurrentTime(2);
     verifyStyle(text, "font-size",    "40px");
-    verifyStyle(rect, "stroke-width", "40");
+    verifyStyle(rect, "stroke-width", "40px");
     svg.setCurrentTime(3);
     verifyStyle(text, "font-size",    "40px");
-    verifyStyle(rect, "stroke-width", "40");
+    verifyStyle(rect, "stroke-width", "40px");
   } catch (e) {
     // If anything goes wrong, make sure we restore textZoom before bubbling
     // the exception upwards, so that we don't mess up subsequent tests.
     SpecialPowers.setTextZoom(window, origTextZoom);
 
     throw e;
   }