Bug 1390367 - Pass SMIL mochitests of stroke-* even if a computed value has no px unit. r?birtles draft
authorMantaroh Yoshinaga <mantaroh@gmail.com>
Tue, 05 Sep 2017 13:15:32 +0900
changeset 658892 8ede5394d12b6583817bd07af255045183d97e0c
parent 658358 8e05298328da75f3056a9f1f9609938870d756a0
child 729793 b190f242c195a2cd2ff311db439e3174e0b7c60c
push id77920
push userbmo:mantaroh@gmail.com
push dateTue, 05 Sep 2017 04:15:45 +0000
reviewersbirtles
bugs1390367, 1379908
milestone57.0a1
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;
   }