Bug 1428244 - Part 2: Set correct oldValue/newValue for the attributeChangedCallback which is fired from style attribute change; draft
authorEdgar Chen <echen@mozilla.com>
Fri, 15 Dec 2017 11:52:07 -0600
changeset 716243 64f39833307a577408034afc7f6fcae5d0f1e27a
parent 716150 015dc9aaa2c799ca442a2994dcec5ea8f133e00c
child 717143 5e2356d8ffa29e5c45e8c60dd9fe83ca4f7a8de1
child 717559 90daa39d31d8b3c65151d5a2eaef9a7fcffbbee5
child 718318 a9dfe67e34c6fa20087d742050cbd0858f4c1c04
push id94369
push userechen@mozilla.com
push dateFri, 05 Jan 2018 10:41:02 +0000
bugs1428244
milestone59.0a1
Bug 1428244 - Part 2: Set correct oldValue/newValue for the attributeChangedCallback which is fired from style attribute change; MozReview-Commit-ID: 4l6XuCUHUh8
dom/base/Element.cpp
dom/base/nsStyledElement.cpp
dom/tests/mochitest/webcomponents/test_custom_element_lifecycle.html
testing/web-platform/meta/custom-elements/reactions/CSSStyleDeclaration.html.ini
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -2683,17 +2683,17 @@ Element::SetAttrAndNotify(int32_t aNames
                           const mozAutoDocUpdate&)
 {
   nsresult rv;
   nsMutationGuard::DidMutate();
 
   // Copy aParsedValue for later use since it will be lost when we call
   // SetAndSwapMappedAttr below
   nsAttrValue valueForAfterSetAttr;
-  if (aCallAfterSetAttr) {
+  if (aCallAfterSetAttr || GetCustomElementData()) {
     valueForAfterSetAttr.SetTo(aParsedValue);
   }
 
   bool hadValidDir = false;
   bool hadDirAuto = false;
   bool oldValueSet;
 
   if (aNamespaceID == kNameSpaceID_None) {
--- a/dom/base/nsStyledElement.cpp
+++ b/dom/base/nsStyledElement.cpp
@@ -81,17 +81,17 @@ nsStyledElement::SetInlineStyleDeclarati
     nsContentUtils::HasMutationListeners(this,
                                          NS_EVENT_BITS_MUTATION_ATTRMODIFIED,
                                          this);
 
   // There's no point in comparing the stylerule pointers since we're always
   // getting a new stylerule here. And we can't compare the stringvalues of
   // the old and the new rules since both will point to the same declaration
   // and thus will be the same.
-  if (hasListeners) {
+  if (hasListeners || GetCustomElementData()) {
     // save the old attribute so we can set up the mutation event properly
     nsAutoString oldValueStr;
     modification = GetAttr(kNameSpaceID_None, nsGkAtoms::style,
                            oldValueStr);
     if (modification) {
       oldValue.SetTo(oldValueStr);
       oldValueSet = true;
     }
--- a/dom/tests/mochitest/webcomponents/test_custom_element_lifecycle.html
+++ b/dom/tests/mochitest/webcomponents/test_custom_element_lifecycle.html
@@ -287,16 +287,58 @@ function testAttributeChangedExtended() 
 
   customElements.define("x-extended-attribute-change", ExtnededAttributeChange,
                         { extends: "button" });
 
   var elem = document.createElement("button", {is: "x-extended-attribute-change"});
   elem.setAttribute("foo", "bar");
 }
 
+function testStyleAttributeChange() {
+  var expectedCallbackArguments = [
+    // [name, oldValue, newValue]
+    // This is an additional attributeChangedCallback from *first* style
+    // attribute change, see https://bugzilla.mozilla.org/show_bug.cgi?id=1428246.
+    ["style", null, ""],
+    ["style", "", "font-size: 10px;"],
+    ["style", "font-size: 10px;", "font-size: 20px;"],
+    ["style", "font-size: 20px;", "font-size: 30px;"],
+  ];
+
+  customElements.define("x-style-attribute-change", class extends HTMLElement {
+    attributeChangedCallback(name, oldValue, newValue) {
+      if (expectedCallbackArguments.length === 0) {
+        ok(false, "Got unexpected attributeChangedCallback?");
+        return;
+      }
+
+      let expectedArgument = expectedCallbackArguments.shift();
+      is(name, expectedArgument[0],
+         "The name argument should match the expected value.");
+      is(oldValue, expectedArgument[1],
+         "The old value argument should match the expected value.");
+      is(newValue, expectedArgument[2],
+         "The new value argument should match the expected value.");
+    }
+
+    static get observedAttributes() {
+      return ["style"];
+    }
+  });
+
+  var elem = document.createElement("x-style-attribute-change");
+  elem.style.fontSize = "10px";
+  elem.style.fontSize = "20px";
+  elem.style.fontSize = "30px";
+
+  ok(expectedCallbackArguments.length === 0,
+     "The attributeChangedCallback should be fired synchronously.");
+  runNextTest();
+}
+
 // Creates a custom element that is an upgrade candidate (no registration)
 // and mutate the element in ways that would call callbacks for registered
 // elements.
 function testUpgradeCandidate() {
   var createdElement = document.createElement("x-upgrade-candidate");
   container.appendChild(createdElement);
   createdElement.setAttribute("foo", "bar");
   container.removeChild(createdElement);
@@ -363,16 +405,17 @@ var testFunctions = [
   testRegisterUnresolvedExtended,
   testInnerHTML,
   testInnerHTMLExtended,
   testInnerHTMLUpgrade,
   testInnerHTMLExtendedUpgrade,
   testRegisterResolved,
   testAttributeChanged,
   testAttributeChangedExtended,
+  testStyleAttributeChange,
   testUpgradeCandidate,
   testChangingCallback,
   testNotInDocEnterLeave,
   testEnterLeaveView,
   SimpleTest.finish
 ];
 
 function runNextTest() {
--- a/testing/web-platform/meta/custom-elements/reactions/CSSStyleDeclaration.html.ini
+++ b/testing/web-platform/meta/custom-elements/reactions/CSSStyleDeclaration.html.ini
@@ -7,19 +7,16 @@
     expected: FAIL
 
   [setProperty on CSSStyleDeclaration must enqueue an attributeChanged reaction when it adds the observed style attribute]
     expected: FAIL
 
   [setProperty on CSSStyleDeclaration must enqueue an attributeChanged reaction when it mutates the observed style attribute]
     expected: FAIL
 
-  [setProperty on CSSStyleDeclaration must enqueue an attributeChanged reaction when it makes a property important and the style attribute is observed]
-    expected: FAIL
-
   [setPropertyValue on CSSStyleDeclaration must enqueue an attributeChanged reaction when it adds the observed style attribute]
     expected: FAIL
 
   [setPropertyValue on CSSStyleDeclaration must not enqueue an attributeChanged reaction when it adds the style attribute but the style attribute is not observed]
     expected: FAIL
 
   [setPropertyValue on CSSStyleDeclaration must enqueue an attributeChanged reaction when it mutates the observed style attribute]
     expected: FAIL
@@ -28,19 +25,16 @@
     expected: FAIL
 
   [setPropertyPriority on CSSStyleDeclaration must enqueue an attributeChanged reaction when it makes a property important and the style attribute is observed]
     expected: FAIL
 
   [setPropertyPriority on CSSStyleDeclaration must enqueue an attributeChanged reaction when it makes a property important but the style attribute is not observed]
     expected: FAIL
 
-  [removeProperty on CSSStyleDeclaration must enqueue an attributeChanged reaction when it removes a property from the observed style attribute]
-    expected: FAIL
-
   [cssFloat on CSSStyleDeclaration must enqueue an attributeChanged reaction when it adds the observed style attribute]
     expected: FAIL
 
   [A camel case attribute (borderWidth) on CSSStyleDeclaration must enqueue an attributeChanged reaction when it adds the observed style attribute]
     expected: FAIL
 
   [A camel case attribute (borderWidth) on CSSStyleDeclaration must enqueue an attributeChanged reaction when it mutates the observed style attribute]
     expected: FAIL