Bug 1545865 Handle inherited attributes overridden by derived element classes r=bgrins
authorAndrew Swan <aswan@mozilla.com>
Sat, 20 Apr 2019 16:22:10 +0000
changeset 470295 581755bf88997188199042fd77a45336bda434d9
parent 470294 bbdaf05c9d16b95a9bea1bf1125d69fb58aacd81
child 470301 72be82c6809e2cc187e5cffdff0c3e686c564a57
push id35893
push userdvarga@mozilla.com
push dateSat, 20 Apr 2019 21:44:54 +0000
treeherdermozilla-central@581755bf8899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1545865
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 1545865 Handle inherited attributes overridden by derived element classes r=bgrins The existing implementation of inherited attributes keeps a data structure in a property on the class, but derived classes were unintentionally getting that structure from their parent class, preventing derived classes from declaring a different set of inherited attributes. Differential Revision: https://phabricator.services.mozilla.com/D28255
toolkit/content/customElements.js
toolkit/content/tests/chrome/test_custom_element_base.xul
--- a/toolkit/content/customElements.js
+++ b/toolkit/content/customElements.js
@@ -230,37 +230,44 @@ MozElements.MozElementMixin = Base => {
    *
    * @return {Object<string selector, string attributes>}
    */
   static get inheritedAttributes() {
     return null;
   }
 
   static get flippedInheritedAttributes() {
-    let {inheritedAttributes} = this;
-    if (!inheritedAttributes) {
-      return null;
-    }
-    if (!this._flippedInheritedAttributes) {
-      this._flippedInheritedAttributes = {};
-      for (let selector in inheritedAttributes) {
-        let attrRules = inheritedAttributes[selector].split(",");
-        for (let attrRule of attrRules) {
-          let attrName = attrRule;
-          let attrNewName = attrRule;
-          let split = attrName.split("=");
-          if (split.length == 2) {
-            attrName = split[1];
-            attrNewName = split[0];
+    // Have to be careful here, if a subclass overrides inheritedAttributes
+    // and its parent class is instantiated first, then reading
+    // this._flippedInheritedAttributes on the child class will return the
+    // computed value from the parent.  We store it separately on each class
+    // to ensure everything works correctly when inheritedAttributes is
+    // overridden.
+    if (!this.hasOwnProperty("_flippedInheritedAttributes")) {
+      let {inheritedAttributes} = this;
+      if (!inheritedAttributes) {
+        this._flippedInheritedAttributes = null;
+      } else {
+        this._flippedInheritedAttributes = {};
+        for (let selector in inheritedAttributes) {
+          let attrRules = inheritedAttributes[selector].split(",");
+          for (let attrRule of attrRules) {
+            let attrName = attrRule;
+            let attrNewName = attrRule;
+            let split = attrName.split("=");
+            if (split.length == 2) {
+              attrName = split[1];
+              attrNewName = split[0];
+            }
+
+            if (!this._flippedInheritedAttributes[attrName]) {
+              this._flippedInheritedAttributes[attrName] = [];
+            }
+            this._flippedInheritedAttributes[attrName].push([selector, attrNewName]);
           }
-
-          if (!this._flippedInheritedAttributes[attrName]) {
-            this._flippedInheritedAttributes[attrName] = [];
-          }
-          this._flippedInheritedAttributes[attrName].push([selector, attrNewName]);
         }
       }
     }
 
     return this._flippedInheritedAttributes;
   }
   /*
    * Generate this array based on `inheritedAttributes`, if any. A class is free to override
--- a/toolkit/content/tests/chrome/test_custom_element_base.xul
+++ b/toolkit/content/tests/chrome/test_custom_element_base.xul
@@ -12,16 +12,17 @@
   <!-- test results are displayed in the html:body -->
   <body xmlns="http://www.w3.org/1999/xhtml" style="height: 300px; overflow: auto;"/>
 
   <button id="one"/>
   <simpleelement id="two" style="-moz-user-focus: normal;"/>
   <simpleelement id="three" disabled="true" style="-moz-user-focus: normal;"/>
   <button id="four"/>
   <inherited-element-declarative foo="fuagra" empty-string=""></inherited-element-declarative>
+  <inherited-element-derived foo="fuagra"></inherited-element-derived>
   <inherited-element-shadowdom-declarative foo="fuagra" empty-string=""></inherited-element-shadowdom-declarative>
   <inherited-element-imperative foo="fuagra" empty-string=""></inherited-element-imperative>
   <inherited-element-beforedomloaded foo="fuagra" empty-string=""></inherited-element-beforedomloaded>
 
   <!-- test code running before page load goes here -->
   <script type="application/javascript"><![CDATA[
     class InheritAttrsChangeBeforDOMLoaded extends MozXULElement {
       static get inheritedAttributes() {
@@ -136,16 +137,23 @@
         this.label = this.querySelector("label");
         this.initializeAttributeInheritance();
       }
     }
     customElements.define("inherited-element-declarative", InheritsElementDeclarative);
     let declarativeEl = document.querySelector("inherited-element-declarative");
     ok(declarativeEl, "declarative inheritance element exists");
 
+    class InheritsElementDerived extends InheritsElementDeclarative {
+      static get inheritedAttributes() {
+        return { label: "renamedfoo=foo" };
+      }
+    }
+    customElements.define("inherited-element-derived", InheritsElementDerived);
+
     class InheritsElementShadowDOMDeclarative extends MozXULElement {
       static get inheritedAttributes() {
         return {
           "label": "text=label,foo,empty-string,bardo=bar",
           "unmatched": "foo", // Make sure we don't throw on unmatched selectors
         };
       }
 
@@ -228,16 +236,23 @@
       el.removeAttribute("bar");
       ok(!el.label.hasAttribute("bardo"),
         "attribute inheritance: does apply when host attr has been removed");
 
       el.setAttribute("bar", "changed-from-host-2");
       is(el.label.getAttribute("bardo"), "changed-from-host-2",
         "attribute inheritance: does apply when host attr has changed after being removed");
     }
+
+    let derivedEl = document.querySelector("inherited-element-derived");
+    ok(derivedEl, "derived inheritance element exists");
+    ok(!derivedEl.label.hasAttribute("foo"),
+       "attribute inheritance: base class attribute is not applied in derived class that overrides it");
+    ok(derivedEl.label.hasAttribute("renamedfoo"),
+       "attribute inheritance: attribute defined in derived class is present");
   }
 
   async function testCustomInterface() {
     class SimpleElement extends MozXULElement {
       get disabled() {
         return this.getAttribute("disabled") == "true";
       }