Bug 1453148: Let overflow parse two values. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 10 Apr 2018 18:40:22 +0200
changeset 412972 eaf30a1f59eedf65d4d6a5009bebfd014873d64e
parent 412971 06f67baf2aed4715a995bfd41a98833d984de27f
child 412973 b12325c786ba621118ddfcb9dcc59bb24136fe7f
push id33828
push userarchaeopteryx@coole-files.de
push dateThu, 12 Apr 2018 19:19:41 +0000
treeherdermozilla-central@6e22c4a726c2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1453148
milestone61.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 1453148: Let overflow parse two values. r=xidorn Per https://github.com/w3c/csswg-drafts/issues/2484. MozReview-Commit-ID: D7M3PhnTtD2
layout/style/nsComputedDOMStyle.cpp
layout/style/test/property_database.js
layout/style/test/test_bug319381.html
servo/components/style/properties/shorthand/box.mako.rs
testing/web-platform/meta/MANIFEST.json
testing/web-platform/meta/css/cssom/index-002.html.ini
testing/web-platform/tests/css/css-overflow/overflow-shorthand-001.html
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -5114,26 +5114,32 @@ nsComputedDOMStyle::DoGetWillChange()
   return valueList.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetOverflow()
 {
   const nsStyleDisplay* display = StyleDisplay();
 
-  if (display->mOverflowX != display->mOverflowY) {
-    // No value to return.  We can't express this combination of
-    // values as a shorthand.
-    return nullptr;
-  }
-
-  RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
-  val->SetIdent(nsCSSProps::ValueToKeywordEnum(display->mOverflowX,
-                                               nsCSSProps::kOverflowKTable));
-  return val.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::DoGetOverflowX()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
   val->SetIdent(
     nsCSSProps::ValueToKeywordEnum(StyleDisplay()->mOverflowX,
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -4203,18 +4203,20 @@ var gCSSProperties = {
   },
   "overflow": {
     domProp: "overflow",
     inherited: false,
     type: CSS_TYPE_SHORTHAND_AND_LONGHAND,
     prerequisites: { "display": "block", "contain": "none" },
     subproperties: [ "overflow-x", "overflow-y" ],
     initial_values: [ "visible" ],
-    other_values: [ "auto", "scroll", "hidden", "-moz-hidden-unscrollable", "-moz-scrollbars-none" ],
-    invalid_values: []
+    other_values: [ "auto", "scroll", "hidden", "-moz-hidden-unscrollable", "-moz-scrollbars-none",
+                    "auto auto", "auto scroll", "hidden scroll", "auto hidden",
+                    "-moz-hidden-unscrollable -moz-hidden-unscrollable" ],
+    invalid_values: [ "-moz-hidden-unscrollable -moz-scrollbars-none" ]
   },
   "overflow-x": {
     domProp: "overflowX",
     inherited: false,
     type: CSS_TYPE_LONGHAND,
     // No applies_to_placeholder because we have a !important rule in forms.css.
     prerequisites: { "display": "block", "overflow-y": "visible", "contain": "none" },
     initial_values: [ "visible" ],
--- a/layout/style/test/test_bug319381.html
+++ b/layout/style/test/test_bug319381.html
@@ -38,51 +38,53 @@ for (i = 0; i < vals.length; ++i) {
   is($('t').style.overflow, vals[i], "Roundtrip");
   is(c(), vals[i], "Simple property set");
   isnot(cval(), null, "Simple property as CSSValue");
 }
 
 $('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, "", "Shorthand read directly");
-is(c(), "", "Shorthand computed");
-is(cval(), null, "Shorthand as CSSValue");
+is($('t').style.overflow, "hidden scroll", "Shorthand read directly");
+is(c(), "hidden scroll", "Shorthand computed");
+isnot(cval(), null, "Shorthand as CSSValue");
 
 $('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, "", "Shorthand read directly");
-is(c(), "", "Shorthand computed");
-is(cval(), null, "Shorthand as CSSValue");
+is($('t').style.overflow, "scroll hidden", "Shorthand read directly");
+is(c(), "scroll hidden", "Shorthand computed");
+isnot(cval(), null, "Shorthand as CSSValue");
 
 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, "", "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");
       isnot(cval(), null, "Shorthand computation as CSSValue");
     } else if (vals[j] == "visible" && vals[i] == "auto") {
       is(c(), vals[i], "Shorthand computation");
-      isnot(cval(), null, "Shorthand computation as CSSValue");    
+      isnot(cval(), null, "Shorthand computation as CSSValue");
     } else {
-      is(c(), "", "Shorthand computation");
-      is(cval(), null, "Shorthand computation as CSSValue");
+      let x = vals[i] == "visible" ? "auto" : vals[i];
+      let y = vals[j] == "visible" ? "auto" : vals[j];
+      is(c(), x + " " + y, "Shorthand computation");
+      isnot(cval(), null, "Shorthand computation as CSSValue");
     }
   }
 }
 
 </script>
 </pre>
 </body>
 </html>
--- a/servo/components/style/properties/shorthand/box.mako.rs
+++ b/servo/components/style/properties/shorthand/box.mako.rs
@@ -1,16 +1,19 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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" sub_properties="overflow-x overflow-y"
-                    spec="https://drafts.csswg.org/css-overflow/#propdef-overflow">
+<%helpers:shorthand
+    name="overflow"
+    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>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
@@ -37,30 +40,33 @@
                         })
                     }
                 }
             });
             if moz_kw_found.is_ok() {
                 return moz_kw_found
             }
         % endif
-        let overflow = parse_overflow(context, input)?;
+        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,
-            overflow_y: overflow,
+            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 {
-            if self.overflow_x == self.overflow_y {
-                self.overflow_x.to_css(dest)
-            } else {
-                Ok(())
+            self.overflow_x.to_css(dest)?;
+            if self.overflow_x != self.overflow_y {
+                dest.write_char(' ')?;
+                self.overflow_y.to_css(dest)?;
             }
+            Ok(())
         }
     }
 </%helpers:shorthand>
 
 <%helpers:shorthand
     name="overflow-clip-box"
     sub_properties="overflow-clip-box-block overflow-clip-box-inline"
     enabled_in="ua"
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -313618,16 +313618,22 @@
     ]
    ],
    "css/css-multicol/multicol-gap-percentage-001.html": [
     [
      "/css/css-multicol/multicol-gap-percentage-001.html",
      {}
     ]
    ],
+   "css/css-overflow/overflow-shorthand-001.html": [
+    [
+     "/css/css-overflow/overflow-shorthand-001.html",
+     {}
+    ]
+   ],
    "css/css-position/position-sticky-bottom.html": [
     [
      "/css/css-position/position-sticky-bottom.html",
      {}
     ]
    ],
    "css/css-position/position-sticky-get-bounding-client-rect.html": [
     [
@@ -503525,16 +503531,20 @@
   "css/css-namespaces/syntax-016.xml": [
    "0cba1aed016d08e4706bffb8a4f4169c9cfd2108",
    "visual"
   ],
   "css/css-overflow/input-scrollable-region-001.html": [
    "f51bc673da28b0471018cdf945b4449ab00ce717",
    "reftest"
   ],
+  "css/css-overflow/overflow-shorthand-001.html": [
+   "6409ee499d3e853d8f4933f1b532e12ed9ab406b",
+   "testharness"
+  ],
   "css/css-overflow/reference/input-scrollable-region-001-ref.html": [
    "31e24bb1a2cb6f42703cc05e055fcb345c770a22",
    "support"
   ],
   "css/css-page/OWNERS": [
    "d4ee6029cc9c064e0e8b2c76becf3b59c4f7b62b",
    "support"
   ],
--- a/testing/web-platform/meta/css/cssom/index-002.html.ini
+++ b/testing/web-platform/meta/css/cssom/index-002.html.ini
@@ -21,17 +21,14 @@
     expected: FAIL
 
   [border is expected to be border-width: 1px; border-top-color: red;]
     expected: FAIL
 
   [border is expected to be border: dotted;]
     expected: FAIL
 
-  [overflow is expected to be overflow: scroll hidden;]
-    expected: FAIL
-
   [outline is expected to be outline: blue dotted 2px;]
     expected: FAIL
 
   [list is expected to be list-style: circle inside;]
     expected: FAIL
 
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-overflow/overflow-shorthand-001.html
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<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(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 : `${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 = `${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: ${x} ${y} works`);
+}
+
+let OVERFLOW_VALUES = [ "auto", "hidden", "scroll", "visible" ];
+for (let x of OVERFLOW_VALUES)
+  for (let y of OVERFLOW_VALUES)
+    testOverflowShorthand(x, y);
+</script>