Implement CSSStyleRule.selectorText.

We parse when assigning using the namespaces of the stylesheet. It isn't clear if the spec says to do that (Firefox doesn't support the setter at all, Chrome does, Safari doesn't); the spec issue is here: w3c/csswg-drafts#1511 Also fix ToCss implementation of AttrSelectorOperator to not pad with spaces, to conform with CSSOM. This means we have to update some unit tests that expect operators with spaces around them in attribute selectors to roundtrip. See the "attribute selector" section of "Serializing Selectors" here: https://drafts.csswg.org/cssom/#serializing-selectors CSSStyleRule.selectorText is specified here: https://drafts.csswg.org/cssom/#dom-cssstylerule-selectortext
servo · Jul 12, 2017 · 92ec8f1 · 92ec8f1
1 parent f857788
commit 92ec8f1
Show file tree

Hide file tree

Showing 14 changed files with 109 additions and 100 deletions.
diff --git a/components/script/dom/cssstyledeclaration.rs b/components/script/dom/cssstyledeclaration.rs
@@ -104,6 +104,8 @@ impl CSSStyleOwner {
                     f(&mut *pdb.write_with(&mut guard), &mut changed)
                 };
                 if changed {
+                    // If this is changed, see also
+                    // CSSStyleRule::SetSelectorText, which does the same thing.
                     rule.global().as_window().Document().invalidate_stylesheets();
                 }
                 result

diff --git a/components/script/dom/cssstylerule.rs b/components/script/dom/cssstylerule.rs
@@ -2,7 +2,10 @@
  * 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/. */
 
+use cssparser::{Parser as CssParser, ParserInput as CssParserInput};
+use cssparser::ToCss;
 use dom::bindings::codegen::Bindings::CSSStyleRuleBinding::{self, CSSStyleRuleMethods};
+use dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods;
 use dom::bindings::inheritance::Castable;
 use dom::bindings::js::{JS, MutNullableJS, Root};
 use dom::bindings::reflector::{DomObject, reflect_dom_object};
@@ -12,9 +15,12 @@ use dom::cssstyledeclaration::{CSSModificationAccess, CSSStyleDeclaration, CSSSt
 use dom::cssstylesheet::CSSStyleSheet;
 use dom::window::Window;
 use dom_struct::dom_struct;
+use selectors::parser::SelectorList;
+use std::mem;
+use style::selector_parser::SelectorParser;
 use style::shared_lock::{Locked, ToCssWithGuard};
 use style::stylearc::Arc;
-use style::stylesheets::StyleRule;
+use style::stylesheets::{StyleRule, Origin};
 
 #[dom_struct]
 pub struct CSSStyleRule {
@@ -71,4 +77,33 @@ impl CSSStyleRuleMethods for CSSStyleRule {
             )
         })
     }
+
+    // https://drafts.csswg.org/cssom/#dom-cssstylerule-selectortext
+    fn SelectorText(&self) -> DOMString {
+        let guard = self.cssrule.shared_lock().read();
+        let stylerule = self.stylerule.read_with(&guard);
+        return DOMString::from_string(stylerule.selectors.to_css_string());
+    }
+
+    // https://drafts.csswg.org/cssom/#dom-cssstylerule-selectortext
+    fn SetSelectorText(&self, value: DOMString) {
+        // It's not clear from the spec if we should use the stylesheet's namespaces.
+        // https://github.com/w3c/csswg-drafts/issues/1511
+        let namespaces = self.cssrule.parent_stylesheet().style_stylesheet().contents.namespaces.read();
+        let parser = SelectorParser {
+            stylesheet_origin: Origin::Author,
+            namespaces: &namespaces,
+        };
+        let mut css_parser = CssParserInput::new(&*value);
+        let mut css_parser = CssParser::new(&mut css_parser);
+        if let Ok(mut s) = SelectorList::parse(&parser, &mut css_parser) {
+            // This mirrors what we do in CSSStyleOwner::mutate_associated_block.
+            let mut guard = self.cssrule.shared_lock().write();
+            let mut stylerule = self.stylerule.write_with(&mut guard);
+            mem::swap(&mut stylerule.selectors, &mut s);
+            // It seems like we will want to avoid having to invalidate all
+            // stylesheets eventually!
+            self.global().as_window().Document().invalidate_stylesheets();
+        }
+    }
 }
diff --git a/components/script/dom/webidls/CSSStyleRule.webidl b/components/script/dom/webidls/CSSStyleRule.webidl
@@ -5,6 +5,6 @@
 // https://drafts.csswg.org/cssom/#the-cssstylerule-interface
 [Exposed=Window]
 interface CSSStyleRule : CSSRule {
-  // attribute DOMString selectorText;
+  attribute DOMString selectorText;
   [SameObject, PutForwards=cssText] readonly attribute CSSStyleDeclaration style;
 };
diff --git a/components/selectors/attr.rs b/components/selectors/attr.rs
@@ -78,13 +78,15 @@ pub enum AttrSelectorOperator {
 
 impl ToCss for AttrSelectorOperator {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+        // https://drafts.csswg.org/cssom/#serializing-selectors
+        // See "attribute selector".
         dest.write_str(match *self {
-            AttrSelectorOperator::Equal => " = ",
-            AttrSelectorOperator::Includes => " ~= ",
-            AttrSelectorOperator::DashMatch => " |= ",
-            AttrSelectorOperator::Prefix => " ^= ",
-            AttrSelectorOperator::Substring => " *= ",
-            AttrSelectorOperator::Suffix => " $= ",
+            AttrSelectorOperator::Equal => "=",
+            AttrSelectorOperator::Includes => "~=",
+            AttrSelectorOperator::DashMatch => "|=",
+            AttrSelectorOperator::Prefix => "^=",
+            AttrSelectorOperator::Substring => "*=",
+            AttrSelectorOperator::Suffix => "$=",
         })
     }
 }

diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs
@@ -1934,7 +1934,7 @@ pub mod tests {
                 ].into_boxed_slice())
             ), specificity(0, 0, 1))
         ))));
-        assert_eq!(parse("[attr |= \"foo\"]"), Ok(SelectorList::from_vec(vec!(
+        assert_eq!(parse("[attr|=\"foo\"]"), Ok(SelectorList::from_vec(vec!(
             Selector::from_vec(vec!(
                 Component::AttributeInNoNamespace {
                     local_name: DummyAtom::from("attr"),
@@ -1993,7 +1993,7 @@ pub mod tests {
         parser.default_ns = None;
         assert!(parse(":not(#provel.old)").is_err());
         assert!(parse(":not(#provel > old)").is_err());
-        assert!(parse("table[rules]:not([rules = \"none\"]):not([rules = \"\"])").is_ok());
+        assert!(parse("table[rules]:not([rules=\"none\"]):not([rules=\"\"])").is_ok());
         assert_eq!(parse(":not(#provel)"), Ok(SelectorList::from_vec(vec!(
             Selector::from_vec(vec!(Component::Negation(vec!(
                     Component::ID(DummyAtom::from("provel")),

diff --git a/tests/unit/style/parsing/selectors.rs b/tests/unit/style/parsing/selectors.rs
@@ -23,5 +23,5 @@ fn test_selectors() {
     assert_roundtrip!(parse_selector, "div");
     assert_roundtrip!(parse_selector, "svg|circle");
     assert_roundtrip!(parse_selector, "p:before", "p::before");
-    assert_roundtrip!(parse_selector, "[border = \"0\"]:-servo-nonzero-border ~ ::-servo-details-summary");
+    assert_roundtrip!(parse_selector, "[border=\"0\"]:-servo-nonzero-border ~ ::-servo-details-summary");
 }
diff --git a/tests/wpt/metadata-css/cssom-1_dev/html/cssstylerule.htm.ini b/tests/wpt/metadata-css/cssom-1_dev/html/cssstylerule.htm.ini
@@ -15,6 +15,3 @@
   [StyleRule_properties]
     expected: FAIL
 
-  [StyleRule_properties_values]
-    expected: FAIL
-
diff --git a/tests/wpt/metadata-css/cssom-1_dev/html/interfaces.htm.ini b/tests/wpt/metadata-css/cssom-1_dev/html/interfaces.htm.ini
@@ -117,12 +117,6 @@
   [CSSRule interface: attribute parentRule]
     expected: FAIL
 
-  [CSSStyleRule interface: attribute selectorText]
-    expected: FAIL
-
-  [CSSStyleRule interface: style_element.sheet.cssRules[0\] must inherit property "selectorText" with the proper type (0)]
-    expected: FAIL
-
   [CSSRule interface: style_element.sheet.cssRules[0\] must inherit property "parentRule" with the proper type (10)]
     expected: FAIL
 

diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json
@@ -160103,6 +160103,18 @@
      {}
     ]
    ],
+   "cssom/selectorText-modification-restyle-001.html": [
+    [
+     "/cssom/selectorText-modification-restyle-001.html",
+     [
+      [
+       "/cssom/selectorText-modification-restyle-001-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
    "html/dom/elements/global-attributes/dir_auto-EN-L.html": [
     [
      "/html/dom/elements/global-attributes/dir_auto-EN-L.html",
@@ -274610,6 +274622,11 @@
      {}
     ]
    ],
+   "cssom/selectorText-modification-restyle-001-ref.html": [
+    [
+     {}
+    ]
+   ],
    "cssom/stylesheet-same-origin.css": [
     [
      {}
@@ -553977,6 +553994,14 @@
    "002777c7c598eb1131ab625365ee3fe08650e830",
    "testharness"
   ],
+  "cssom/selectorText-modification-restyle-001-ref.html": [
+   "4fbe124f09131f30e5c68154774246e5cd15b14a",
+   "support"
+  ],
+  "cssom/selectorText-modification-restyle-001.html": [
+   "ac5a5414dd849151c3ca6348c90c0f4e80a09b75",
+   "reftest"
+  ],
   "cssom/serialization-CSSDeclaration-with-important.html": [
    "ecc8b95fb2d71cacee271f4fea2fc16f35cdba57",
    "testharness"

diff --git a/tests/wpt/metadata/cssom/CSSStyleRule.html.ini b/tests/wpt/metadata/cssom/CSSStyleRule.html.ini
@@ -9,9 +9,3 @@
   [Values of CSSRule attributes]
     expected: FAIL
 
-  [Values of CSSStyleRule attributes]
-    expected: FAIL
-
-  [Existence and type of CSSStyleRule attributes]
-    expected: FAIL
-
diff --git a/tests/wpt/metadata/cssom/interfaces.html.ini b/tests/wpt/metadata/cssom/interfaces.html.ini
@@ -54,12 +54,6 @@
   [CSSRule interface: attribute parentRule]
     expected: FAIL
 
-  [CSSStyleRule interface: attribute selectorText]
-    expected: FAIL
-
-  [CSSStyleRule interface: style_element.sheet.cssRules[0\] must inherit property "selectorText" with the proper type (0)]
-    expected: FAIL
-
   [CSSRule interface: style_element.sheet.cssRules[0\] must inherit property "parentRule" with the proper type (10)]
     expected: FAIL
 

diff --git a/tests/wpt/metadata/selectors/attribute-selectors/attribute-case/cssom.html.ini b/tests/wpt/metadata/selectors/attribute-selectors/attribute-case/cssom.html.ini
diff --git a/tests/wpt/web-platform-tests/cssom/selectorText-modification-restyle-001-ref.html b/tests/wpt/web-platform-tests/cssom/selectorText-modification-restyle-001-ref.html
@@ -0,0 +1,13 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>(Ref #1) CSSOM - CSSStyleRule.selectorText Modification Restyle - Reference #1</title>
+
+<style>
+div {
+  color: green;
+}
+</style>
+
+<body>
+<div>I should be green.</div>
+</body>
diff --git a/tests/wpt/web-platform-tests/cssom/selectorText-modification-restyle-001.html b/tests/wpt/web-platform-tests/cssom/selectorText-modification-restyle-001.html
@@ -0,0 +1,21 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>(Test #1) CSSOM - CSSStyleRule.selectorText Modification Restyle - Test #1</title>
+<link rel="match" href="selectorText-modification-restyle-001-ref.html">
+
+<style>
+@namespace bogus url(http://example.com/bogus);
+
+bogus|div {
+  color: green;
+}
+</style>
+
+<body>
+<div>I should be green.</div>
+<script>
+// Remove the "bogus" namespace--now it should apply to the div above.
+// We also expect to see a restyle.
+document.querySelector("style").sheet.cssRules[1].selectorText = "div";
+</script>
+</body>