Bug 1545823 - Implement non-standard CSSStyleSheet.rules, CSSStyleSheet.addRule and CSSStyleSheet.removeRule. r=bzbarsky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 09 May 2019 12:32:52 +0000
changeset 532018 a93ed2a80220b9ba15fb9b59ea79b5f1ee8f7693
parent 532017 921e77da2c82a59ba12cd7bca42fc39a824c31e8
child 532019 41acb048244d7d2bad371761bf54c80670fe683b
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1545823
milestone68.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 1545823 - Implement non-standard CSSStyleSheet.rules, CSSStyleSheet.addRule and CSSStyleSheet.removeRule. r=bzbarsky It's not worth dying on this hill. Both Blink and WebKit pass the tests. (Well, WebKit actually fails one of the latest ones I wrote, cssRules and rules are not the same JS object, WebKit returns a new rule list. I'll file) Spec PR in https://github.com/w3c/csswg-drafts/pull/3900. Differential Revision: https://phabricator.services.mozilla.com/D30348
dom/webidl/CSSStyleSheet.webidl
layout/style/StyleSheet.cpp
layout/style/StyleSheet.h
testing/web-platform/meta/css/css-counter-styles/idlharness.html.ini
testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/declared.tentative.html.ini
testing/web-platform/tests/css/cssom/CSSStyleSheet.html
--- a/dom/webidl/CSSStyleSheet.webidl
+++ b/dom/webidl/CSSStyleSheet.webidl
@@ -19,9 +19,17 @@ interface CSSStyleSheet : StyleSheet {
   [Throws, NeedsSubjectPrincipal]
   readonly attribute CSSRuleList cssRules;
   [ChromeOnly, BinaryName="parsingModeDOM"]
   readonly attribute CSSStyleSheetParsingMode parsingMode;
   [Throws, NeedsSubjectPrincipal]
   unsigned long insertRule(DOMString rule, optional unsigned long index = 0);
   [Throws, NeedsSubjectPrincipal]
   void deleteRule(unsigned long index);
+
+  // Non-standard WebKit things.
+  [Throws, NeedsSubjectPrincipal, BinaryName="cssRules"]
+  readonly attribute CSSRuleList rules;
+  [Throws, NeedsSubjectPrincipal, BinaryName="deleteRule"]
+  void removeRule(optional unsigned long index = 0);
+  [Throws, NeedsSubjectPrincipal]
+  long addRule(optional DOMString selector = "undefined", optional DOMString style = "undefined", optional unsigned long index);
 };
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -505,36 +505,54 @@ void StyleSheet::SetSourceURL(const nsAS
   mInner->mSourceURL = aSourceURL;
 }
 
 css::Rule* StyleSheet::GetDOMOwnerRule() const { return mOwnerRule; }
 
 uint32_t StyleSheet::InsertRule(const nsAString& aRule, uint32_t aIndex,
                                 nsIPrincipal& aSubjectPrincipal,
                                 ErrorResult& aRv) {
-  if (IsReadOnly()) {
-    return 0;
-  }
-  if (!AreRulesAvailable(aSubjectPrincipal, aRv)) {
+  if (IsReadOnly() || !AreRulesAvailable(aSubjectPrincipal, aRv)) {
     return 0;
   }
   return InsertRuleInternal(aRule, aIndex, aRv);
 }
 
 void StyleSheet::DeleteRule(uint32_t aIndex, nsIPrincipal& aSubjectPrincipal,
                             ErrorResult& aRv) {
-  if (IsReadOnly()) {
-    return;
-  }
-  if (!AreRulesAvailable(aSubjectPrincipal, aRv)) {
+  if (IsReadOnly() || !AreRulesAvailable(aSubjectPrincipal, aRv)) {
     return;
   }
   return DeleteRuleInternal(aIndex, aRv);
 }
 
+int32_t StyleSheet::AddRule(const nsAString& aSelector, const nsAString& aBlock,
+                            const Optional<uint32_t>& aIndex,
+                            nsIPrincipal& aSubjectPrincipal, ErrorResult& aRv) {
+  if (IsReadOnly() || !AreRulesAvailable(aSubjectPrincipal, aRv)) {
+    return -1;
+  }
+
+  nsAutoString rule;
+  rule.Append(aSelector);
+  rule.AppendLiteral(" { ");
+  if (!aBlock.IsEmpty()) {
+    rule.Append(aBlock);
+    rule.Append(' ');
+  }
+  rule.Append('}');
+
+  auto index =
+      aIndex.WasPassed() ? aIndex.Value() : GetCssRulesInternal()->Length();
+
+  InsertRuleInternal(rule, index, aRv);
+  // Always return -1.
+  return -1;
+}
+
 nsresult StyleSheet::DeleteRuleFromGroup(css::GroupRule* aGroup,
                                          uint32_t aIndex) {
   NS_ENSURE_ARG_POINTER(aGroup);
   NS_ASSERTION(IsComplete(), "No deleting from an incomplete sheet!");
   RefPtr<css::Rule> rule = aGroup->GetStyleRuleAt(aIndex);
   NS_ENSURE_TRUE(rule, NS_ERROR_ILLEGAL_VALUE);
 
   // check that the rule actually belongs to this sheet!
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -341,16 +341,19 @@ class StyleSheet final : public nsICSSLo
   // called GetOwnerRule because that would be ambiguous with the ImportRule
   // version.
   css::Rule* GetDOMOwnerRule() const;
   dom::CSSRuleList* GetCssRules(nsIPrincipal& aSubjectPrincipal, ErrorResult&);
   uint32_t InsertRule(const nsAString& aRule, uint32_t aIndex,
                       nsIPrincipal& aSubjectPrincipal, ErrorResult& aRv);
   void DeleteRule(uint32_t aIndex, nsIPrincipal& aSubjectPrincipal,
                   ErrorResult& aRv);
+  int32_t AddRule(const nsAString& aSelector, const nsAString& aBlock,
+                  const dom::Optional<uint32_t>& aIndex,
+                  nsIPrincipal& aSubjectPrincipal, ErrorResult& aRv);
 
   // WebIDL miscellaneous bits
   inline dom::ParentObject GetParentObject() const;
   JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) final;
 
   // Changes to sheets should be after a WillDirty call.
   void WillDirty();
 
--- a/testing/web-platform/meta/css/css-counter-styles/idlharness.html.ini
+++ b/testing/web-platform/meta/css/css-counter-styles/idlharness.html.ini
@@ -1,46 +1,4 @@
 [idlharness.html]
   [css-counter-styles IDL tests]
     expected: FAIL
 
-  [CSSCounterStyleRule interface: counter must inherit property "speakAs" with the proper type]
-    expected: FAIL
-
-  [Stringification of counter]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "symbols" with the proper type]
-    expected: FAIL
-
-  [CSSRule interface: counter must inherit property "COUNTER_STYLE_RULE" with the proper type]
-    expected: FAIL
-
-  [CSSCounterStyleRule must be primary interface of counter]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "pad" with the proper type]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "prefix" with the proper type]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "suffix" with the proper type]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "name" with the proper type]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "fallback" with the proper type]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "negative" with the proper type]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "additiveSymbols" with the proper type]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "range" with the proper type]
-    expected: FAIL
-
-  [CSSCounterStyleRule interface: counter must inherit property "system" with the proper type]
-    expected: FAIL
-
--- a/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/declared.tentative.html.ini
+++ b/testing/web-platform/meta/css/css-typed-om/the-stylepropertymap/declared/declared.tentative.html.ini
@@ -1,4 +1,25 @@
 [declared.tentative.html]
   [Declared StylePropertyMap tests]
     expected: FAIL
 
+  [Declared StylePropertyMap contains custom property declarations]
+    expected: FAIL
+
+  [Declared StylePropertyMap contains properties with their last valid value]
+    expected: FAIL
+
+  [Declared StylePropertyMap does not contain properties with invalid values]
+    expected: FAIL
+
+  [Declared StylePropertyMap only contains properties in the style rule]
+    expected: FAIL
+
+  [Declared StylePropertyMap is live]
+    expected: FAIL
+
+  [Declared StylePropertyMap does not contain inline styles]
+    expected: FAIL
+
+  [Declared StylePropertyMap contains CSS property declarations in style rules]
+    expected: FAIL
+
--- a/testing/web-platform/tests/css/cssom/CSSStyleSheet.html
+++ b/testing/web-platform/tests/css/cssom/CSSStyleSheet.html
@@ -33,12 +33,38 @@
 
         styleSheet.deleteRule(1);
         assert_equals(styleSheet.cssRules.length, 2, "CSSStyleSheet cssRules attribute after deleteRule function");
         assert_equals(styleSheet.cssRules[0].cssText, "body { width: 50%; }", "CSSStyleSheet cssRules attribute after deleteRule function");
         assert_equals(styleSheet.cssRules[1].cssText, "#foo { height: 100px; }", "CSSStyleSheet cssRules attribute after deleteRule function");
         assert_equals(styleSheet.cssRules[2], undefined, "CSSStyleSheet cssRules attribute after deleteRule function");
         assert_equals(styleSheet.cssRules[0].randomProperty, 1, "[SameObject] cssRules attribute after deleteRule function");
         assert_equals(styleSheet.cssRules[1].randomProperty, 2, "[SameObject] cssRules attribute after deleteRule function");
+
+        styleSheet.removeRule();
+        assert_equals(styleSheet.cssRules.length, 1, "CSSStyleSheet cssRules attribute after removeRule function");
+        assert_equals(styleSheet.cssRules[0].cssText, "#foo { height: 100px; }", "CSSStyleSheet cssRules attribute after removeRule function");
+
+        assert_equals(styleSheet.addRule("@media all", "#foo { color: red }"), -1);
+        assert_equals(styleSheet.cssRules.length, 2, "CSSStyleSheet cssRules attribute after addRule function");
+        assert_true(styleSheet.cssRules[1] instanceof CSSMediaRule, "CSSStyleSheet addRule does some silly string concatenation");
+
+        styleSheet.removeRule(1);
+        assert_equals(styleSheet.cssRules.length, 1, "CSSStyleSheet cssRules attribute after removeRule function with index");
+        assert_equals(styleSheet.cssRules[0].cssText, "#foo { height: 100px; }", "CSSStyleSheet cssRules attribute after deleteRule function with index");
+
+        assert_equals(styleSheet.addRule("#foo", "color: red"), -1);
+        assert_equals(styleSheet.cssRules.length, 2, "CSSStyleSheet cssRules attribute after addRule function with simple selector");
+        assert_equals(styleSheet.cssRules[1].cssText, "#foo { color: red; }", "CSSStyleSheet cssRules attribute after addRule function without index appends to the end");
+
+        assert_equals(styleSheet.addRule("#foo", "color: blue", 0), -1);
+        assert_equals(styleSheet.cssRules.length, 3, "CSSStyleSheet cssRules attribute after addRule function with simple selector with index");
+        assert_equals(styleSheet.cssRules[0].cssText, "#foo { color: blue; }", "addRule function with index performs an insertion");
+
+        assert_equals(styleSheet.addRule(), -1);
+        assert_equals(styleSheet.cssRules.length, 4, "CSSStyleSheet cssRules attribute after addRule function without arguments");
+        assert_equals(styleSheet.cssRules[3].cssText, "undefined { }", "addRule arguments default to undefined");
+
+        assert_equals(styleSheet.cssRules, styleSheet.rules, "CSSStyleSheet.rules returns the same object as CSSStyleSheet.cssRules");
     });
     </script>
 </head>
 </html>