Bug 666041 patch 4.5: Add support for custom flex container sizes to test_flexbox_layout.html, and give it some testcases that exercise our float-accumulation-error-handling code. r=dbaron (DONTBUILD because this test is currently disabled on trunk)
authorDaniel Holbert <dholbert@cs.stanford.edu>
Tue, 11 Sep 2012 18:07:49 -0700
changeset 106860 a32dd4305ba8ded20a192ac331a6b73bd4b7894f
parent 106859 0558ede9693e57663e4836c7e678b55465ddc2ab
child 106861 f04c43188e4700b8355850b08c616aa0c82a4b8a
push id14720
push userdholbert@mozilla.com
push dateWed, 12 Sep 2012 01:08:41 +0000
treeherdermozilla-inbound@a32dd4305ba8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs666041
milestone18.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 666041 patch 4.5: Add support for custom flex container sizes to test_flexbox_layout.html, and give it some testcases that exercise our float-accumulation-error-handling code. r=dbaron (DONTBUILD because this test is currently disabled on trunk)
layout/style/test/flexbox_layout_testcases.js
layout/style/test/test_flexbox_layout.html
--- a/layout/style/test/flexbox_layout_testcases.js
+++ b/layout/style/test/flexbox_layout_testcases.js
@@ -15,20 +15,25 @@
  *  (a) the property's specified value
  *  ...or...
  *  (b) an array with 2 entries: [specifiedValue, expectedComputedValue] if the
  *      property's computed value is intended to be checked. The first entry
  *      (for the specified value) may be null; this means that no value should
  *      be explicitly specified for this property.
  *
  * To allow these testcases to be re-used in both horizontal and vertical
- * flex containers, we specify "width"/"min-width"/etc. as "_main-size",
- * "_min-main-size", etc.  The test code can map these placeholder names to
- * their corresponding property-names using the maps defined below --
- * gHorizontalPropertyMapping and gVerticalPropertyMapping.
+ * flex containers, we specify "width"/"min-width"/etc. using the aliases
+ * "_main-size", "_min-main-size", etc.  The test code can map these
+ * placeholder names to their corresponding property-names using the maps
+ * defined below -- gRowPropertyMapping, gColumnPropertyMapping, etc.
+ *
+ * If the testcase needs to customize its flex container at all (e.g. by
+ * specifying a custom container-size), it can do so by including a hash
+ * called "container_properties", with propertyName:propertyValue mappings.
+ * (This hash can use aliased property-names like "_main-size" as well.)
  */
 
 // The standard main-size we'll use for our flex container when setting up
 // the testcases defined below:
 var gDefaultFlexContainerSize = "200px";
 
 // Left-to-right versions of placeholder property-names used in
 // testcases below:
@@ -291,16 +296,189 @@ var gFlexboxTestcases =
        },
        {
          "-moz-flex": "99999999999999999999999999999999999",
          "_main-size": [ null,  "50px" ]
        },
      ]
  },
 
+ // And now, some testcases to check that we handle float accumulation error
+ // gracefully.
+
+ // First, a testcase with just a custom-sized huge container, to be sure we'll
+ // be able to handle content on that scale, in the subsequent more-complex
+ // testcases:
+ {
+   container_properties:
+   {
+     "_main-size": "9000000px"
+   },
+   items:
+     [
+       {
+         "-moz-flex": "1",
+         "_main-size": [ null,  "9000000px" ]
+       },
+     ]
+ },
+ // ...and now with two flex items dividing up that container's huge size:
+ {
+   container_properties:
+   {
+     "_main-size": "9000000px"
+   },
+   items:
+     [
+       {
+         "-moz-flex": "2",
+         "_main-size": [ null,  "6000000px" ]
+       },
+       {
+         "-moz-flex": "1",
+         "_main-size": [ null,  "3000000px" ]
+       },
+     ]
+ },
+
+ // OK, now to actually test accumulation error. Below, we have six flex items
+ // splitting up the container's size, with huge differences between flex
+ // weights.  For simplicity, I've set up the weights so that they sum exactly
+ // to the container's size in px. So 1 unit of flex *should* get you 1px.
+ //
+ // NOTE: The expected computed "_main-size" values for the flex items below
+ // appear to add up to more than their container's size, which would suggest
+ // that they overflow their container unnecessarily. But they don't actually
+ // overflow -- this discrepancy is simply because Gecko's code for reporting
+ // computed-sizes rounds to 6 significant figures (in particular, the method
+ // (nsTSubstring_CharT::AppendFloat() does this).  Internally, in app-units,
+ // the child frames' main-sizes add up exactly to the container's main-size,
+ // as you'd hope & expect.
+ {
+   container_properties:
+   {
+     "_main-size": "9000000px"
+   },
+   items:
+     [
+       {
+         "-moz-flex": "3000000",
+         "_main-size": [ null,  "3000000px" ]
+       },
+       {
+         "-moz-flex": "1",
+         "_main-size": [ null,  "1px" ]
+       },
+       {
+         "-moz-flex": "1",
+         "_main-size": [ null,  "1px" ]
+       },
+       {
+         "-moz-flex": "2999999",
+         // NOTE: Expected value is off slightly, from float error when
+         // resolving flexible lengths & when generating computed value string:
+         "_main-size": [ null,  "3000000px" ]
+       },
+       {
+         "-moz-flex": "2999998",
+         // NOTE: Expected value is off slightly, from float error when
+         // resolving flexible lengths & when generating computed value string:
+         "_main-size": [ null,  "3000000px" ]
+       },
+       {
+         "-moz-flex": "1",
+         "_main-size": [ null,  "1px" ]
+       },
+     ]
+ },
+ // Same flex items as previous testcase, but now reordered such that the items
+ // with tiny flex weights are all listed last:
+ {
+   container_properties:
+   {
+     "_main-size": "9000000px"
+   },
+   items:
+     [
+       {
+         "-moz-flex": "3000000",
+         "_main-size": [ null,  "3000000px" ]
+       },
+       {
+         "-moz-flex": "2999999",
+         // NOTE: Expected value is off slightly, from float error when
+         // resolving flexible lengths & when generating computed value string:
+         "_main-size": [ null,  "3000000px" ]
+       },
+       {
+         "-moz-flex": "2999998",
+         // NOTE: Expected value is off slightly, from float error when
+         // resolving flexible lengths & when generating computed value string:
+         "_main-size": [ null,  "3000000px" ]
+       },
+       {
+         "-moz-flex": "1",
+         "_main-size": [ null,  "1px" ]
+       },
+       {
+         "-moz-flex": "1",
+         "_main-size": [ null,  "1px" ]
+       },
+       {
+         "-moz-flex": "1",
+         "_main-size": [ null,  "1px" ]
+       },
+     ]
+ },
+ // Same flex items as previous testcase, but now reordered such that the items
+ // with tiny flex weights are all listed first:
+ {
+   container_properties:
+   {
+     "_main-size": "9000000px"
+   },
+   items:
+     [
+       {
+         "-moz-flex": "1",
+         // NOTE: Expected value is off slightly, from float error when
+         // resolving flexible lengths:
+         "_main-size": [ null,  "0.966667px" ]
+       },
+       {
+         "-moz-flex": "1",
+         // NOTE: Expected value is off slightly, from float error when
+         // resolving flexible lengths:
+         "_main-size": [ null,  "0.983333px" ]
+       },
+       {
+         "-moz-flex": "1",
+         // NOTE: Expected value is off slightly, from float error when
+         // resolving flexible lengths:
+         "_main-size": [ null,  "0.983333px" ]
+       },
+       {
+         "-moz-flex": "3000000",
+         "_main-size": [ null,  "3000000px" ]
+       },
+       {
+         "-moz-flex": "2999999",
+         // NOTE: Expected value is off slightly, from float error when
+         // resolving flexible lengths & when generating computed value string:
+         "_main-size": [ null,  "3000000px" ]
+       },
+       {
+         "-moz-flex": "2999998",
+         // NOTE: Expected value is off slightly, from float error when
+         // resolving flexible lengths & when generating computed value string:
+         "_main-size": [ null,  "3000000px" ]
+       },
+     ]
+ },
+
  // Trying "flex: auto" (== "1 1 auto") w/ a mix of flex-grow/flex-basis values
  {
    items:
      [
        {
          "-moz-flex": "auto",
          "_main-size": [ null, "45px" ]
        },
--- a/layout/style/test/test_flexbox_layout.html
+++ b/layout/style/test/test_flexbox_layout.html
@@ -32,29 +32,51 @@ https://bugzilla.mozilla.org/show_bug.cg
 
 SimpleTest.waitForExplicitFinish();
 
 function getComputedStyleWrapper(elem, prop)
 {
   return window.getComputedStyle(elem, null).getPropertyValue(prop);
 }
 
+function setPossiblyAliasedProperty(aElem, aPropertyName, aPropertyValue,
+                                    aPropertyMapping)
+{
+  let actualPropertyName = (aPropertyName in aPropertyMapping ?
+                            aPropertyMapping[aPropertyName] : aPropertyName);
+
+  if (!gCSSProperties[actualPropertyName]) {
+    ok(false, "Bug in test: property '" + actualPropertyName +
+              "' doesn't exist in gCSSProperties");
+  } else {
+    let domPropertyName = gCSSProperties[actualPropertyName].domProp;
+    aElem.style[domPropertyName] = aPropertyValue;
+  }
+}
+
 // The main test function.
 // aFlexboxTestcase is an entry from the list in flexbox_layout_testcases.js
 function testFlexboxTestcase(aFlexboxTestcase, aFlexDirection, aPropertyMapping)
 {
   let content = document.getElementById("content");
 
   // Create flex container
   let flexContainer = document.createElement("div");
   flexContainer.style.display = "-moz-flex";
+  flexContainer.style.MozFlexDirection = aFlexDirection;
+  setPossiblyAliasedProperty(flexContainer, "_main-size", gDefaultFlexContainerSize, aPropertyMapping);
 
-  flexContainer.style[aPropertyMapping["_main-size"]] = gDefaultFlexContainerSize;
-
-  flexContainer.style.MozFlexDirection = aFlexDirection;
+  // Apply testcase's customizations for flex container (if any).
+  if (aFlexboxTestcase.container_properties) {
+    for (let propName in aFlexboxTestcase.container_properties) {
+      let propValue = aFlexboxTestcase.container_properties[propName];
+      setPossiblyAliasedProperty(flexContainer, propName, propValue,
+                                 aPropertyMapping);
+    }
+  }
 
   // Create & append flex items
   aFlexboxTestcase.items.forEach(function(aChildSpec) {
     // Create an element for our item
     let child = document.createElement("div");
 
     // Set all the specified properties on our item
     for (let propName in aChildSpec) {
@@ -65,24 +87,19 @@ function testFlexboxTestcase(aFlexboxTes
         aChildSpec[propName];
 
       // SANITY CHECK:
       if (Array.isArray(aChildSpec[propName])) {
         is(aChildSpec[propName].length, 2,
            "unexpected number of elements in array within child spec");
       }
 
-      let actualPropName = (propName in aPropertyMapping ?
-                            aPropertyMapping[propName] : propName);
-      if (!gCSSProperties[actualPropName]) {
-        ok(false, "Bug in test: property '" + actualPropName +
-                  "' doesn't exist in gCSSProperties");
-      } else if (specifiedValue !== null) {
-        let domPropName = gCSSProperties[actualPropName].domProp;
-        child.style[domPropName] = specifiedValue;
+      if (specifiedValue !== null) {
+        setPossiblyAliasedProperty(child, propName, specifiedValue,
+                                   aPropertyMapping);
       }
     }
 
     // Append the item to the flex container
     flexContainer.appendChild(child);
   });
 
   // Append the flex container