Bug 1451940: Don't reparse stylesheets when assigning empty string to empty element. r=bz a=jcristau
authorKris Maglione <maglione.k@gmail.com>
Tue, 10 Apr 2018 11:06:26 -0700
changeset 463202 84d5902b3a0276cf83cb0d3fa742d2721935f59b
parent 463201 36b90debcd465bc6c7af1c3b01ba73aef7647611
child 463203 99690fed70cc2a25e1b78efd2ec445969e92f3e7
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, jcristau
bugs1451940
milestone60.0
Bug 1451940: Don't reparse stylesheets when assigning empty string to empty element. r=bz a=jcristau Per spec, assinging an empty string to textContent or innerHTML should not trigger any mutation observers, and therefore should not trigger a stylesheet reparse, when the element is already empty. Since we need to apply some special handling to innerHTML and textContent assignments to <style> nodes, and therefore re-parse directly in the setter rather than in response to mutation observers, we need to special-case this scenario. MozReview-Commit-ID: KdOYFs8ayT7
dom/html/HTMLStyleElement.cpp
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/css/cssom/css-style-reparse.html
--- a/dom/html/HTMLStyleElement.cpp
+++ b/dom/html/HTMLStyleElement.cpp
@@ -173,16 +173,26 @@ HTMLStyleElement::SetInnerHTML(const nsA
   SetTextContentInternal(aInnerHTML, aScriptedPrincipal, aError);
 }
 
 void
 HTMLStyleElement::SetTextContentInternal(const nsAString& aTextContent,
                                          nsIPrincipal* aScriptedPrincipal,
                                          ErrorResult& aError)
 {
+  // Per spec, if we're setting text content to an empty string and don't
+  // already have any children, we should not trigger any mutation observers, or
+  // re-parse the stylesheet.
+  if (aTextContent.IsEmpty() && !GetFirstChild()) {
+    nsIPrincipal* principal = mTriggeringPrincipal ? mTriggeringPrincipal.get() : NodePrincipal();
+    if (principal == aScriptedPrincipal) {
+      return;
+    }
+  }
+
   SetEnableUpdates(false);
 
   aError = nsContentUtils::SetNodeTextContent(this, aTextContent, true);
 
   SetEnableUpdates(true);
 
   mTriggeringPrincipal = aScriptedPrincipal;
 
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -314300,16 +314300,22 @@
     ]
    ],
    "css/cssom/css-style-declaration-modifications.html": [
     [
      "/css/cssom/css-style-declaration-modifications.html",
      {}
     ]
    ],
+   "css/cssom/css-style-reparse.html": [
+    [
+     "/css/cssom/css-style-reparse.html",
+     {}
+    ]
+   ],
    "css/cssom/cssimportrule.html": [
     [
      "/css/cssom/cssimportrule.html",
      {}
     ]
    ],
    "css/cssom/cssom-cssText-serialize.html": [
     [
@@ -525619,16 +525625,20 @@
   "css/cssom/css-style-attribute-modifications.html": [
    "9199534f3b6cc473832562b1701ade3a05dde172",
    "testharness"
   ],
   "css/cssom/css-style-declaration-modifications.html": [
    "c169d758c1d91b75697b04cf72750f8ac1650e1a",
    "testharness"
   ],
+  "css/cssom/css-style-reparse.html": [
+   "d83b1fde05df64628f67b7773757c12d213f566b",
+   "testharness"
+  ],
   "css/cssom/cssimportrule.html": [
    "f8a110a236529e78b528117b25866015d67568d9",
    "testharness"
   ],
   "css/cssom/cssom-cssText-serialize.html": [
    "66ad91da39c1e1da9021f6443e9b6d34baf57dcb",
    "testharness"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/cssom/css-style-reparse.html
@@ -0,0 +1,59 @@
+<!doctype html>
+<html>
+<head>
+  <meta charset=utf-8>
+  <title>CSS Test: DOM modification re-parsing test</title>
+  <link rel="help" href="https://drafts.csswg.org/cssom/">
+  <link rel="help" href="http://www.w3.org/TR/cssom-1/#the-cssrule-interface">
+  <script src=/resources/testharness.js></script>
+  <script src=/resources/testharnessreport.js></script>
+  <style>div { min-width: 0px; }</style>
+  <style id="style-element"></style>
+</head>
+<body>
+<div id="test-div"></div>
+<script type="text/javascript">
+    var style = document.getElementById("style-element");
+    var div = document.getElementById("test-div");
+
+    function testProperty(prop) {
+      // Assigning an empty string to textContent or innerHTML should trigger a
+      // reparse only if the element is not empty.
+      style.sheet.insertRule("#test-div { min-width: 42px; }");
+      assert_equals(getComputedStyle(div).minWidth, "42px");
+
+      style[prop] = "";
+      assert_equals(getComputedStyle(div).minWidth, "42px");
+
+      style[prop] = " ";
+      assert_equals(getComputedStyle(div).minWidth, "0px");
+
+      style.sheet.insertRule("#test-div { min-width: 42px; }");
+      assert_equals(getComputedStyle(div).minWidth, "42px");
+
+      style[prop] = "";
+      assert_equals(getComputedStyle(div).minWidth, "0px");
+
+      style.sheet.insertRule("#test-div { min-width: 42px; }");
+      assert_equals(getComputedStyle(div).minWidth, "42px");
+
+      style.appendChild(document.createTextNode(""));
+      assert_equals(getComputedStyle(div).minWidth, "0px");
+
+      style.sheet.insertRule("#test-div { min-width: 42px; }");
+      assert_equals(getComputedStyle(div).minWidth, "42px");
+
+      style[prop] = "";
+      assert_equals(getComputedStyle(div).minWidth, "0px");
+    }
+
+    test(function() {
+      testProperty("textContent");
+    }, "style.textContent modification");
+
+    test(function() {
+      testProperty("innerHTML");
+    }, "style.innerHTML modification");
+</script>
+</body>
+</html>