Bug 1492567 - Back out bug 1481866. r=dbaron a=pascalc
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 19 Sep 2018 22:02:01 +0200
changeset 492589 862f7fc8be9db739acc0c48b8b49f63ff4213079
parent 492588 a53244dbea631c9d08c97aeb687460210baff126
child 492590 6f98c616ab30175f3129a6132d41e51cdc4bc796
push id1815
push userffxbld-merge
push dateMon, 15 Oct 2018 10:40:45 +0000
treeherdermozilla-release@18d4c09e9378 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron, pascalc
bugs1492567, 1481866, 1453148
milestone63.0
Bug 1492567 - Back out bug 1481866. r=dbaron a=pascalc Summary: The behavior the WG proposed is way more subtle than what that bug implements, including: * Implementing two logical overflow longhands. * Expanding the overflow shorthand to different longhands depending on the syntax of that. Meanwhile, Blink hasn't done the swap and will ship the same behavior that we shipped in Firefox 61 (bug 1453148), that is, overflow-x, then overflow-y. So I think lacking a clear way forward we should revert this change and preserve our shipped behavior. Reviewers: dbaron! Tags: #secure-revision Bug #: 1492567 Differential Revision: https://phabricator.services.mozilla.com/D6317
devtools/shared/css/generated/properties-db.js
gfx/tests/crashtests/1325159-1.html
layout/base/tests/preserve3d_sorting_hit_testing2_iframe.html
layout/forms/crashtests/367587-1.html
layout/mathml/crashtests/355986-1.xhtml
layout/style/nsComputedDOMStyle.cpp
layout/style/test/test_bug319381.html
servo/components/style/properties/shorthands/box.mako.rs
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/css/css-overflow/overflow-shorthand-001.html
testing/web-platform/tests/css/cssom/overflow-serialization.html
testing/web-platform/tests/css/cssom/shorthand-values.html
--- a/devtools/shared/css/generated/properties-db.js
+++ b/devtools/shared/css/generated/properties-db.js
@@ -7561,18 +7561,18 @@ exports.CSS_PROPERTIES = {
       "thick",
       "thin",
       "unset"
     ]
   },
   "overflow": {
     "isInherited": false,
     "subproperties": [
-      "overflow-y",
-      "overflow-x"
+      "overflow-x",
+      "overflow-y"
     ],
     "supports": [],
     "values": [
       "-moz-hidden-unscrollable",
       "auto",
       "hidden",
       "inherit",
       "initial",
--- a/gfx/tests/crashtests/1325159-1.html
+++ b/gfx/tests/crashtests/1325159-1.html
@@ -15,17 +15,17 @@
 }
 </style>
 <script>
 function boom(){
   let doc = document.documentElement;
   o_2.style.MozBorderEndStyle = "dotted";
   doc.style.MozPerspective = "24.5pt";
   o_0.style.MozTransformStyle = "preserve-3d";
-  doc.style.overflow = "hidden scroll";
+  doc.style.overflow = "scroll hidden";
   doc.style.textOverflow = "''";
   o_0.style.offsetInlineStart = "calc(3*25px)";
   doc.style.paddingTop = "calc(67108864%)";
   doc.style.width = "3e-0%";
   o_0.style.display = "-moz-stack";
   o_0.style.position = "relative";
 }
 addEventListener("DOMContentLoaded", boom);
--- a/layout/base/tests/preserve3d_sorting_hit_testing2_iframe.html
+++ b/layout/base/tests/preserve3d_sorting_hit_testing2_iframe.html
@@ -35,17 +35,17 @@ div {
 }
 
 #container hr {
   margin: 0 20px 0 20px;
 }
 
 #content {
   -ms-overflow-style: none;
-  overflow: scroll hidden;
+  overflow: hidden scroll;
   height: 100%;
   background: #fefee0;
 }
 
 
 #lorem {
   font-size: 7em;
   float: left;
--- a/layout/forms/crashtests/367587-1.html
+++ b/layout/forms/crashtests/367587-1.html
@@ -1,16 +1,16 @@
 <html class="reftest-wait">
 <head>
 
 <style>
 
 #opt1 { display: table-footer-group; }
 #opt1 { visibility: collapse; }
-#opt1 { overflow: scroll hidden; }
+#opt1 { overflow: hidden scroll; }
 
 #opt2 { display: table-cell; }
 #opt2 { clear: left; }
 
 select { width: 1px; }
 
 </style>
 
--- a/layout/mathml/crashtests/355986-1.xhtml
+++ b/layout/mathml/crashtests/355986-1.xhtml
@@ -1,17 +1,17 @@
 <html xmlns="http://www.w3.org/1999/xhtml">
 
 
 <head>
 <style>
 
 body * { 
   display: table-footer-group; 
-  overflow: scroll hidden;
+  overflow: hidden scroll;
 }
 
 </style>
 </head>
 
 
 
 <body onload="document.getElementById('eee').style.display = 'inline';">
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -3851,31 +3851,31 @@ nsComputedDOMStyle::DoGetWillChange()
   return valueList.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetOverflow()
 {
   const nsStyleDisplay* display = StyleDisplay();
 
-  RefPtr<nsROCSSPrimitiveValue> overflowY = new nsROCSSPrimitiveValue;
-  overflowY->SetIdent(
-    nsCSSProps::ValueToKeywordEnum(display->mOverflowY,
-                                   nsCSSProps::kOverflowKTable));
-  if (display->mOverflowX == display->mOverflowY) {
-    return overflowY.forget();
-  }
-  RefPtr<nsDOMCSSValueList> valueList = GetROCSSValueList(false);
-  valueList->AppendCSSValue(overflowY.forget());
-
   RefPtr<nsROCSSPrimitiveValue> overflowX = new nsROCSSPrimitiveValue;
   overflowX->SetIdent(
     nsCSSProps::ValueToKeywordEnum(display->mOverflowX,
                                    nsCSSProps::kOverflowKTable));
+  if (display->mOverflowX == display->mOverflowY) {
+    return overflowX.forget();
+  }
+  RefPtr<nsDOMCSSValueList> valueList = GetROCSSValueList(false);
   valueList->AppendCSSValue(overflowX.forget());
+
+  RefPtr<nsROCSSPrimitiveValue> overflowY= new nsROCSSPrimitiveValue;
+  overflowY->SetIdent(
+    nsCSSProps::ValueToKeywordEnum(display->mOverflowY,
+                                   nsCSSProps::kOverflowKTable));
+  valueList->AppendCSSValue(overflowY.forget());
   return valueList.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetOverflowY()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
   val->SetIdent(
--- a/layout/style/test/test_bug319381.html
+++ b/layout/style/test/test_bug319381.html
@@ -33,49 +33,49 @@ for (i = 0; i < vals.length; ++i) {
   is($('t').style.overflow, vals[i], "Roundtrip");
   is(c(), vals[i], "Simple property set");
 }
 
 if (SpecialPowers.getBoolPref("layout.css.overflow.moz-scrollbars.enabled")) {
   $('t').style.overflow = mozVals[0];
   is($('t').style.getPropertyValue("overflow-y"), "scroll", "Roundtrip");
   is($('t').style.getPropertyValue("overflow-x"), "hidden", "Roundtrip");
-  is($('t').style.overflow, "scroll hidden", "Shorthand read directly");
-  is(c(), "scroll hidden", "Shorthand computed");
+  is($('t').style.overflow, "hidden scroll", "Shorthand read directly");
+  is(c(), "hidden scroll", "Shorthand computed");
 
   $('t').style.overflow = mozVals[1];
   is($('t').style.getPropertyValue("overflow-x"), "scroll", "Roundtrip");
   is($('t').style.getPropertyValue("overflow-y"), "hidden", "Roundtrip");
-  is($('t').style.overflow, "hidden scroll", "Shorthand read directly");
-  is(c(), "hidden scroll", "Shorthand computed");
+  is($('t').style.overflow, "scroll hidden", "Shorthand read directly");
+  is(c(), "scroll hidden", "Shorthand computed");
 }
 
 for (i = 0; i < vals.length; ++i) {
   for (j = 0; j < vals.length; ++j) {
     $('t').setAttribute("style",
                         "overflow-x: " + vals[i] + "; overflow-y: " + vals[j]);
     is($('t').style.getPropertyValue("overflow-x"), vals[i], "Roundtrip");
     is($('t').style.getPropertyValue("overflow-y"), vals[j], "Roundtrip");
 
     if (i == j) {
       is($('t').style.overflow, vals[i], "Shorthand serialization");
     } else {
-      is($('t').style.overflow, vals[j] + " " + vals[i], "Shorthand serialization");
+      is($('t').style.overflow, vals[i] + " " + vals[j], "Shorthand serialization");
     }
 
     // "visible" overflow-x and overflow-y become "auto" in computed style if
     // the other direction is not also "visible".
     if (i == j || (vals[i] == "visible" && vals[j] == "auto")) {
       is(c(), vals[j], "Shorthand computation");
     } else if (vals[j] == "visible" && vals[i] == "auto") {
       is(c(), vals[i], "Shorthand computation");
     } else {
       let x = vals[i] == "visible" ? "auto" : vals[i];
       let y = vals[j] == "visible" ? "auto" : vals[j];
-      is(c(), y + " " + x, "Shorthand computation");
+      is(c(), x + " " + y, "Shorthand computation");
     }
   }
 }
 </script>
 </pre>
 </body>
 </html>
 
--- a/servo/components/style/properties/shorthands/box.mako.rs
+++ b/servo/components/style/properties/shorthands/box.mako.rs
@@ -2,17 +2,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 <%helpers:shorthand
     name="overflow"
     flags="SHORTHAND_IN_GETCS"
-    sub_properties="overflow-y overflow-x"
+    sub_properties="overflow-x overflow-y"
     spec="https://drafts.csswg.org/css-overflow/#propdef-overflow"
 >
     use properties::longhands::overflow_x::parse as parse_overflow;
     % if product == "gecko":
         use properties::longhands::overflow_x::SpecifiedValue;
     % endif
 
     pub fn parse_value<'i, 't>(
@@ -47,31 +47,31 @@
                         }
                     }
                 });
                 if moz_kw_found.is_ok() {
                     return moz_kw_found
                 }
             }
         % endif
-        let overflow_y = parse_overflow(context, input)?;
-        let overflow_x =
-            input.try(|i| parse_overflow(context, i)).unwrap_or(overflow_y);
+        let overflow_x = parse_overflow(context, input)?;
+        let overflow_y =
+            input.try(|i| parse_overflow(context, i)).unwrap_or(overflow_x);
         Ok(expanded! {
             overflow_x: overflow_x,
             overflow_y: overflow_y,
         })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
-            self.overflow_y.to_css(dest)?;
+            self.overflow_x.to_css(dest)?;
             if self.overflow_x != self.overflow_y {
                 dest.write_char(' ')?;
-                self.overflow_x.to_css(dest)?;
+                self.overflow_y.to_css(dest)?;
             }
             Ok(())
         }
     }
 </%helpers:shorthand>
 
 <%helpers:shorthand
     name="overflow-clip-box"
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -542062,17 +542062,17 @@
    "15bf57f793c7d73cc1100c6f34dd29328ca30d01",
    "testharness"
   ],
   "css/css-overflow/orthogonal-flow-with-inline-end-margin.html": [
    "534dff9766ddccf4dd239a307d28d7e8882d6f04",
    "testharness"
   ],
   "css/css-overflow/overflow-shorthand-001.html": [
-   "d922e902720ae6a38ff9c644ddd63ddfd5e62c37",
+   "f425636c3bb4297e4e6564d1c2629dc10dde5607",
    "testharness"
   ],
   "css/css-overflow/reference/input-scrollable-region-001-ref.html": [
    "de894fab610b31257d73ef5488c376e50d899fb9",
    "support"
   ],
   "css/css-page/META.yml": [
    "2f9e29e2787e8c41d0a9ebe1df8342a6c52a01a8",
@@ -570394,17 +570394,17 @@
    "3de142c0b46dde2722bcf18386c6ca9154372488",
    "testharness"
   ],
   "css/cssom/shorthand-serialization.html": [
    "2178ba37a959fc56537c7cc164d423d14563881f",
    "testharness"
   ],
   "css/cssom/shorthand-values.html": [
-   "1e4d93acb26e3f644b103974b910203abf93c56e",
+   "d8d7f5349a4561cb4d764f366236e88eb3775c8f",
    "testharness"
   ],
   "css/cssom/style-attr-update-across-documents.html": [
    "2b8f4435202d06cfa8474acdfbe4c1859e1033ae",
    "testharness"
   ],
   "css/cssom/style-sheet-interfaces-001.html": [
    "20dc9713e58b05a159ef731164889fc37b4b02aa",
@@ -570730,17 +570730,17 @@
    "bba0c091a29d94615609d11eb23ce28bf8d57b87",
    "support"
   ],
   "css/filter-effects/fecolormatrix-type.html": [
    "8caaede02f27a120278d9d1512084e6fd57ab27f",
    "reftest"
   ],
   "css/filter-effects/filter-cb-abspos-inline-001-ref.html": [
-   "b4beae8004155c30dad4f48db3e2087f66c42c4f",
+   "6ebe4635511242cd0f5965a778a5a491cc406436",
    "support"
   ],
   "css/filter-effects/filter-cb-abspos-inline-001.html": [
    "6f99c48d5f34761ba1bc1ce7dbdfd927469ac65a",
    "reftest"
   ],
   "css/filter-effects/filter-cb-abspos-inline-002.html": [
    "6fcf8fea160f5661c97be6c6f45e5f3667badf51",
--- a/testing/web-platform/tests/css/css-overflow/overflow-shorthand-001.html
+++ b/testing/web-platform/tests/css/css-overflow/overflow-shorthand-001.html
@@ -3,40 +3,40 @@
 <title>CSS Overflow Test: Overflow longhand accepts two values</title>
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <link rel="author" title="Emilio Cobos Álvarez <emilio@crisal.io>">
 <link rel="help" href="https://drafts.csswg.org/css-overflow/#propdef-overflow">
 <div id="test-div"></div>
 <script>
 let div = document.getElementById("test-div");
-function testOverflowShorthand(y, x) {
+function testOverflowShorthand(x, y) {
   test(function() {
     div.style.overflowX = x;
     div.style.overflowY = y;
 
     let expectedX = getComputedStyle(div).overflowX;
     let expectedY = getComputedStyle(div).overflowY;
-    let expectedComputedSerialization = expectedX == expectedY ? expectedX : `${expectedY} ${expectedX}`;
-    let expectedSpecifiedSerialization = x == y ? x : `${y} ${x}`;
+    let expectedComputedSerialization = expectedX == expectedY ? expectedX : `${expectedX} ${expectedY}`;
+    let expectedSpecifiedSerialization = x == y ? x : `${x} ${y}`;
 
     assert_equals(div.style.overflow, expectedSpecifiedSerialization);
     assert_equals(getComputedStyle(div).overflow, expectedComputedSerialization);
 
     div.style.overflowX = "";
     div.style.overflowY = "";
     assert_equals(div.style.overflow, "");
 
-    div.style.overflow = `${y} ${x}`;
+    div.style.overflow = `${x} ${y}`;
     assert_equals(div.style.overflow, expectedSpecifiedSerialization);
     assert_equals(div.style.overflowX, x);
     assert_equals(div.style.overflowY, y);
     assert_equals(getComputedStyle(div).overflow, expectedComputedSerialization);
     assert_equals(getComputedStyle(div).overflowX, expectedX);
     assert_equals(getComputedStyle(div).overflowY, expectedY);
-  }, `overflow: ${y} ${x} works`);
+  }, `overflow: ${x} ${y} works`);
 }
 
 let OVERFLOW_VALUES = [ "auto", "hidden", "scroll", "visible" ];
 for (let x of OVERFLOW_VALUES)
   for (let y of OVERFLOW_VALUES)
-    testOverflowShorthand(y, x);
+    testOverflowShorthand(x, y);
 </script>
--- a/testing/web-platform/tests/css/cssom/overflow-serialization.html
+++ b/testing/web-platform/tests/css/cssom/overflow-serialization.html
@@ -17,17 +17,17 @@
     <script>
     test(function () {
         var styleSheet = document.styleSheets[0];
 
         assert_equals(styleSheet.cssRules[0].style.cssText, "overflow: inherit;", "Single value overflow with CSS-wide keyword should serialize correctly.");
         assert_equals(styleSheet.cssRules[1].style.cssText, "overflow: hidden;", "Single value overflow with non-CSS-wide keyword should serialize correctly.");
         assert_equals(styleSheet.cssRules[2].style.cssText, "overflow: initial;", "Overflow-x/y longhands with same CSS-wide keyword should serialize correctly.");
         assert_equals(styleSheet.cssRules[3].style.cssText, "overflow: scroll;", "Overflow-x/y longhands with same non-CSS-wide keyword should serialize correctly.");
-        assert_equals(styleSheet.cssRules[4].style.cssText, "overflow: hidden scroll;", "Overflow-x/y longhands with different keywords should serialize correctly.");
+        assert_equals(styleSheet.cssRules[4].style.cssText, "overflow: scroll hidden;", "Overflow-x/y longhands with different keywords should serialize correctly.");
 
         var div = document.createElement('div');
         div.style.overflow = "inherit";
         assert_equals(div.style.overflow, "inherit", "Single value overflow with CSS-wide keyword should serialize correctly.");
 
         div.style.overflow = "hidden";
         assert_equals(div.style.overflow, "hidden", "Single value overflow with non-CSS-wide keyword should serialize correctly.");
 
@@ -37,13 +37,13 @@
         assert_equals(div.style.overflow, "initial", "Overflow-x/y longhands with same CSS-wide keyword should serialize correctly.");
 
         div.style.overflowX = "scroll";
         div.style.overflowY = "scroll";
         assert_equals(div.style.overflow, "scroll", "Overflow-x/y longhands with same non-CSS-wide keyword should serialize correctly.");
 
         div.style.overflowX = "scroll";
         div.style.overflowY = "hidden";
-        assert_equals(div.style.overflow, "hidden scroll", "Overflow-x/y longhands with different keywords should serialize correctly.");
+        assert_equals(div.style.overflow, "scroll hidden", "Overflow-x/y longhands with different keywords should serialize correctly.");
     });
     </script>
 </head>
 </html>
--- a/testing/web-platform/tests/css/cssom/shorthand-values.html
+++ b/testing/web-platform/tests/css/cssom/shorthand-values.html
@@ -27,17 +27,17 @@
         'border: red;': 'border: red;',
         'border-top: 1px; border-right: 1px; border-bottom: 1px; border-left: 1px;': 'border: 1px;',
         'border-top: 1px; border-right: 2px; border-bottom: 3px; border-left: 4px;': 'border-width: 1px 2px 3px 4px;',
         'border: 1px; border-top: 2px;': 'border-width: 2px 1px 1px;',
         'border: 1px; border-top: 1px !important;': 'border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-top-width: 1px !important;',
         'border: 1px; border-top-color: red;': 'border-width: 1px; border-top-color: red;',
         'border: solid; border-style: dotted': 'border: dotted;',
         'border-width: 1px;': 'border-width: 1px;',
-        'overflow-x: scroll; overflow-y: hidden;': 'overflow: hidden scroll;',
+        'overflow-x: scroll; overflow-y: hidden;': 'overflow: scroll hidden;',
         'overflow-x: scroll; overflow-y: scroll;': 'overflow: scroll;',
         'outline-width: 2px; outline-style: dotted; outline-color: blue;': 'outline: blue dotted 2px;',
         'margin-top: 1px; margin-right: 2px; margin-bottom: 3px; margin-left: 4px;': 'margin: 1px 2px 3px 4px;',
         'list-style-type: circle; list-style-position: inside; list-style-image: initial;': 'list-style: circle inside;',
         'list-style-type: lower-alpha;': 'list-style-type: lower-alpha;',
         'font-family: sans-serif; line-height: 2em; font-size: 3em; font-style: italic; font-weight: bold;': 'font-family: sans-serif; line-height: 2em; font-size: 3em; font-style: italic; font-weight: bold;',
         'padding-top: 1px; padding-right: 2px; padding-bottom: 3px; padding-left: 4px;': 'padding: 1px 2px 3px 4px;'
       }