Bug 1334051 - Part 2: Invoke attributeChangedCallback only if attribute name is in the observed attribute list.

We call attributeChangedCallback in two cases:

1. When any of the attributes in the observed attribute list has changed, appended, removed, or replaced.

2. When upgrading an element, for each attribute in element's attribute list that is in the observed attribute list.

Note: w/ Fixup for not implementing an API Enhancement Bug 1363481.

Tag UXP Issue mcp-graveyard/UXP%1344

6 files changed, 197 insertions, 43 deletions

diff --git a/dom/base/CustomElementRegistry.cpp b/dom/base/CustomElementRegistry.cpp
index cc6264b03..34ad9549d 100644
--- a/dom/base/CustomElementRegistry.cpp
+++ b/dom/base/CustomElementRegistry.cpp
@@ -434,8 +434,27 @@ CustomElementRegistry::EnqueueLifecycleCallback(nsIDocument::ElementCallbackType
                                                 LifecycleCallbackArgs* aArgs,
                                                 CustomElementDefinition* aDefinition)
 {
+  RefPtr<CustomElementData> elementData = aCustomElement->GetCustomElementData();
+  MOZ_ASSERT(elementData, "CustomElementData should exist");
+
+  // Let DEFINITION be ELEMENT's definition
+  CustomElementDefinition* definition = aDefinition;
+  if (!definition) {
+    mozilla::dom::NodeInfo* info = aCustomElement->NodeInfo();
+
+    // Make sure we get the correct definition in case the element
+    // is a extended custom element e.g. <button is="x-button">.
+    nsCOMPtr<nsIAtom> typeAtom = elementData ?
+      elementData->mType.get() : info->NameAtom();
+
+    definition = mCustomDefinitions.Get(typeAtom);
+    if (!definition || definition->mLocalName != info->NameAtom()) {
+      return;
+    }
+  }
+
   auto callback =
-    CreateCustomElementCallback(aType, aCustomElement, aArgs, aDefinition);
+    CreateCustomElementCallback(aType, aCustomElement, aArgs, definition);
   if (!callback) {
     return;
   }
@@ -445,9 +464,17 @@ CustomElementRegistry::EnqueueLifecycleCallback(nsIDocument::ElementCallbackType
     return;
   }
 
+  if (aType == nsIDocument::eAttributeChanged) {
+    nsCOMPtr<nsIAtom> attrName = NS_Atomize(aArgs->name);
+    if (definition->mObservedAttributes.IsEmpty() ||
+        !definition->mObservedAttributes.Contains(attrName)) {
+      return;
+    }
+  }
+
   CustomElementReactionsStack* reactionsStack =
     docGroup->CustomElementReactionsStack();
-  reactionsStack->EnqueueCallbackReaction(this, aCustomElement, aDefinition,
+  reactionsStack->EnqueueCallbackReaction(this, aCustomElement, definition,
                                           Move(callback));
 }
 
@@ -657,6 +684,7 @@ CustomElementRegistry::Define(const nsAString& aName,
 
   JS::Rooted<JSObject*> constructorPrototype(cx);
   nsAutoPtr<LifecycleCallbacks> callbacksHolder(new LifecycleCallbacks());
+  nsCOMArray<nsIAtom> observedAttributes;
   { // Set mIsCustomDefinitionRunning.
     /**
      * 9. Set this CustomElementRegistry's element definition is running flag.
@@ -718,6 +746,14 @@ CustomElementRegistry::Define(const nsAString& aName,
         return;
       }
 
+      // Note: We call the init from the constructorProtoUnwrapped's compartment
+      //       here.
+      JS::RootedValue rootedv(cx, JS::ObjectValue(*constructorProtoUnwrapped));
+      if (!JS_WrapValue(cx, &rootedv) || !callbacksHolder->Init(cx, rootedv)) {
+        aRv.StealExceptionFromJSContext(cx);
+        return;
+      }
+
       /**
        * 10.5. Let observedAttributes be an empty sequence<DOMString>.
        * 10.6. If the value of the entry in lifecycleCallbacks with key
@@ -730,14 +766,45 @@ CustomElementRegistry::Define(const nsAString& aName,
        *          any exceptions from the conversion.
        */
       // TODO: Bug 1293921 - Implement connected/disconnected/adopted/attributeChanged lifecycle callbacks for custom elements
+      if (callbacksHolder->mAttributeChangedCallback.WasPassed()) {
+        // Enter constructor's compartment.
+        JSAutoCompartment ac(cx, constructor);
+        JS::Rooted<JS::Value> observedAttributesIterable(cx);
+
+        if (!JS_GetProperty(cx, constructor, "observedAttributes",
+                            &observedAttributesIterable)) {
+          aRv.StealExceptionFromJSContext(cx);
+          return;
+        }
 
-      // Note: We call the init from the constructorProtoUnwrapped's compartment
-      //       here.
-      JS::RootedValue rootedv(cx, JS::ObjectValue(*constructorProtoUnwrapped));
-      if (!JS_WrapValue(cx, &rootedv) || !callbacksHolder->Init(cx, rootedv)) {
-        aRv.StealExceptionFromJSContext(cx);
-        return;
-      }
+        if (!observedAttributesIterable.isUndefined()) {
+          JS::ForOfIterator iter(cx);
+          if (!iter.init(observedAttributesIterable)) {
+            aRv.StealExceptionFromJSContext(cx);
+            return;
+          }
+
+          JS::Rooted<JS::Value> attribute(cx);
+          while (true) {
+            bool done;
+            if (!iter.next(&attribute, &done)) {
+              aRv.StealExceptionFromJSContext(cx);
+              return;
+            }
+            if (done) {
+              break;
+            }
+
+            JSString *attrJSStr = attribute.toString();
+            nsAutoJSString attrStr;
+            if (!attrStr.init(cx, attrJSStr)) {
+              aRv.StealExceptionFromJSContext(cx);
+              return;
+            }
+            observedAttributes.AppendElement(NS_Atomize(attrStr));
+          }
+        }
+      } // Leave constructor's compartment.
     } // Leave constructorProtoUnwrapped's compartment.
   } // Unset mIsCustomDefinitionRunning
 
@@ -754,6 +821,7 @@ CustomElementRegistry::Define(const nsAString& aName,
     new CustomElementDefinition(nameAtom,
                                 localNameAtom,
                                 &aFunctionConstructor,
+                                Move(observedAttributes),
                                 constructorPrototype,
                                 callbacks,
                                 0 /* TODO dependent on HTML imports. Bug 877072 */);
@@ -878,8 +946,35 @@ CustomElementRegistry::Upgrade(Element* aElement,
     return;
   }
 
-  // Step 3 and Step 4.
-  // TODO: Bug 1334051 - Implement list of observed attributes for custom elements' attributeChanged callbacks
+  // Step 3.
+  if (!aDefinition->mObservedAttributes.IsEmpty()) {
+    uint32_t count = aElement->GetAttrCount();
+    for (uint32_t i = 0; i < count; i++) {
+      mozilla::dom::BorrowedAttrInfo info = aElement->GetAttrInfoAt(i);
+
+      const nsAttrName* name = info.mName;
+      nsIAtom* attrName = name->LocalName();
+
+      if (aDefinition->IsInObservedAttributeList(attrName)) {
+        int32_t namespaceID = name->NamespaceID();
+        nsAutoString attrValue, namespaceURI;
+        info.mValue->ToString(attrValue);
+        nsContentUtils::NameSpaceManager()->GetNameSpaceURI(namespaceID,
+                                                            namespaceURI);
+
+        LifecycleCallbackArgs args = {
+          nsDependentAtomString(attrName),
+          NullString(),
+          (attrValue.IsEmpty() ? NullString() : attrValue),
+          (namespaceURI.IsEmpty() ? NullString() : namespaceURI)
+        };
+        EnqueueLifecycleCallback(nsIDocument::eAttributeChanged, aElement,
+                                 &args, aDefinition);
+      }
+    }
+  }
+
+  // Step 4.
   // TODO: Bug 1334043 - Implement connected lifecycle callbacks for custom elements
 
   // Step 5.
@@ -1051,12 +1146,14 @@ CustomElementReactionsStack::InvokeReactions(ElementQueue* aElementQueue,
 CustomElementDefinition::CustomElementDefinition(nsIAtom* aType,
                                                  nsIAtom* aLocalName,
                                                  Function* aConstructor,
+                                                 nsCOMArray<nsIAtom>&& aObservedAttributes,
                                                  JSObject* aPrototype,
                                                  LifecycleCallbacks* aCallbacks,
                                                  uint32_t aDocOrder)
   : mType(aType),
     mLocalName(aLocalName),
     mConstructor(new CustomElementConstructor(aConstructor)),
+    mObservedAttributes(Move(aObservedAttributes)),
     mPrototype(aPrototype),
     mCallbacks(aCallbacks),
     mDocOrder(aDocOrder)
diff --git a/dom/base/CustomElementRegistry.h b/dom/base/CustomElementRegistry.h
index b5903f978..b85d089f9 100644
--- a/dom/base/CustomElementRegistry.h
+++ b/dom/base/CustomElementRegistry.h
@@ -128,6 +128,7 @@ struct CustomElementDefinition
   CustomElementDefinition(nsIAtom* aType,
                           nsIAtom* aLocalName,
                           Function* aConstructor,
+                          nsCOMArray<nsIAtom>&& aObservedAttributes,
                           JSObject* aPrototype,
                           mozilla::dom::LifecycleCallbacks* aCallbacks,
                           uint32_t aDocOrder);
@@ -141,6 +142,9 @@ struct CustomElementDefinition
   // The custom element constructor.
   RefPtr<CustomElementConstructor> mConstructor;
 
+  // The list of attributes that this custom element observes.
+  nsCOMArray<nsIAtom> mObservedAttributes;
+
   // The prototype to use for new custom elements of this type.
   JS::Heap<JSObject *> mPrototype;
 
@@ -153,9 +157,19 @@ struct CustomElementDefinition
   // The document custom element order.
   uint32_t mDocOrder;
 
-  bool IsCustomBuiltIn() {
+  bool IsCustomBuiltIn()
+  {
     return mType != mLocalName;
   }
+
+  bool IsInObservedAttributeList(nsIAtom* aName)
+  {
+    if (mObservedAttributes.IsEmpty()) {
+      return false;
+    }
+
+    return mObservedAttributes.Contains(aName);
+  }
 };
 
 class CustomElementReaction
diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp
index 247fbe79e..5719b423d 100644
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -2586,23 +2586,29 @@ Element::SetAttrAndNotify(int32_t aNamespaceID,
 
   UpdateState(aNotify);
 
-  nsIDocument* ownerDoc = OwnerDoc();
-  if (ownerDoc && GetCustomElementData()) {
-    nsCOMPtr<nsIAtom> oldValueAtom = oldValue->GetAsAtom();
-    nsCOMPtr<nsIAtom> newValueAtom = valueForAfterSetAttr.GetAsAtom();
-    nsAutoString ns;
-    nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNamespaceID, ns);
-
-    LifecycleCallbackArgs args = {
-      nsDependentAtomString(aName),
-      aModType == nsIDOMMutationEvent::ADDITION ?
-        NullString() : nsDependentAtomString(oldValueAtom),
-      nsDependentAtomString(newValueAtom),
-      (ns.IsEmpty() ? NullString() : ns)
-    };
-
-    nsContentUtils::EnqueueLifecycleCallback(
-      ownerDoc, nsIDocument::eAttributeChanged, this, &args);
+  if (nsContentUtils::IsWebComponentsEnabled()) {
+    if (CustomElementData* data = GetCustomElementData()) {
+      if (CustomElementDefinition* definition =
+            nsContentUtils::GetElementDefinitionIfObservingAttr(this,
+                                                                data->mType,
+                                                                aName)) {
+        nsCOMPtr<nsIAtom> oldValueAtom = oldValue->GetAsAtom();
+        nsCOMPtr<nsIAtom> newValueAtom = valueForAfterSetAttr.GetAsAtom();
+        nsAutoString ns;
+        nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNamespaceID, ns);
+
+        LifecycleCallbackArgs args = {
+          nsDependentAtomString(aName),
+          aModType == nsIDOMMutationEvent::ADDITION ?
+            NullString() : nsDependentAtomString(oldValueAtom),
+          nsDependentAtomString(newValueAtom),
+          (ns.IsEmpty() ? NullString() : ns)
+        };
+
+        nsContentUtils::EnqueueLifecycleCallback(
+          OwnerDoc(), nsIDocument::eAttributeChanged, this, &args, definition);
+      }
+    }
   }
 
   if (aCallAfterSetAttr) {
@@ -2847,20 +2853,27 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName,
 
   UpdateState(aNotify);
 
-  nsIDocument* ownerDoc = OwnerDoc();
-  if (ownerDoc && GetCustomElementData()) {
-    nsAutoString ns;
-    nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNameSpaceID, ns);
-    nsCOMPtr<nsIAtom> oldValueAtom = oldValue.GetAsAtom();
-    LifecycleCallbackArgs args = {
-      nsDependentAtomString(aName),
-      nsDependentAtomString(oldValueAtom),
-      NullString(),
-      (ns.IsEmpty() ? NullString() : ns)
-    };
-
-    nsContentUtils::EnqueueLifecycleCallback(
-      ownerDoc, nsIDocument::eAttributeChanged, this, &args);
+  if (nsContentUtils::IsWebComponentsEnabled()) {
+    if (CustomElementData* data = GetCustomElementData()) {
+      if (CustomElementDefinition* definition =
+            nsContentUtils::GetElementDefinitionIfObservingAttr(this,
+                                                                data->mType,
+                                                                aName)) {
+        nsAutoString ns;
+        nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNameSpaceID, ns);
+
+        nsCOMPtr<nsIAtom> oldValueAtom = oldValue.GetAsAtom();
+        LifecycleCallbackArgs args = {
+          nsDependentAtomString(aName),
+          nsDependentAtomString(oldValueAtom),
+          NullString(),
+          (ns.IsEmpty() ? NullString() : ns)
+        };
+
+        nsContentUtils::EnqueueLifecycleCallback(
+          OwnerDoc(), nsIDocument::eAttributeChanged, this, &args, definition);
+      }
+    }
   }
 
   if (aNotify) {
diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp
index b8ee09bf6..570065dba 100644
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -9637,6 +9637,28 @@ nsContentUtils::SetupCustomElement(Element* aElement,
   return registry->SetupCustomElement(aElement, aTypeExtension);
 }
 
+/* static */ CustomElementDefinition*
+nsContentUtils::GetElementDefinitionIfObservingAttr(Element* aCustomElement,
+                                                    nsIAtom* aExtensionType,
+                                                    nsIAtom* aAttrName)
+{
+  nsString extType = nsDependentAtomString(aExtensionType);
+  NodeInfo *ni = aCustomElement->NodeInfo();
+
+  CustomElementDefinition* definition =
+    LookupCustomElementDefinition(aCustomElement->OwnerDoc(), ni->LocalName(),
+                                  ni->NamespaceID(),
+                                  extType.IsEmpty() ? nullptr : &extType);
+
+  // Custom element not defined yet or attribute is not in the observed
+  // attribute list.
+  if (!definition || !definition->IsInObservedAttributeList(aAttrName)) {
+    return nullptr;
+  }
+
+  return definition;
+}
+
 /* static */ void
 nsContentUtils::EnqueueLifecycleCallback(nsIDocument* aDoc,
                                          nsIDocument::ElementCallbackType aType,
diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h
index 06a7b5ded..96c920394 100644
--- a/dom/base/nsContentUtils.h
+++ b/dom/base/nsContentUtils.h
@@ -2723,6 +2723,11 @@ public:
   static void SetupCustomElement(Element* aElement,
                                  const nsAString* aTypeExtension = nullptr);
 
+  static mozilla::dom::CustomElementDefinition*
+  GetElementDefinitionIfObservingAttr(Element* aCustomElement,
+                                      nsIAtom* aExtensionType,
+                                      nsIAtom* aAttrName);
+
   static void EnqueueLifecycleCallback(nsIDocument* aDoc,
                                        nsIDocument::ElementCallbackType aType,
                                        Element* aCustomElement,
diff --git a/dom/tests/mochitest/webcomponents/mochitest.ini b/dom/tests/mochitest/webcomponents/mochitest.ini
index 3ab56de95..76c8abc38 100644
--- a/dom/tests/mochitest/webcomponents/mochitest.ini
+++ b/dom/tests/mochitest/webcomponents/mochitest.ini
@@ -19,6 +19,7 @@ support-files =
   htmlconstructor_builtin_tests.js
 [test_custom_element_import_node_created_callback.html]
 [test_custom_element_in_shadow.html]
+skip-if = true || stylo # disabled - See bug 1390396 and 1293844
 [test_custom_element_register_invalid_callbacks.html]
 [test_custom_element_get.html]
 [test_custom_element_when_defined.html]
@@ -33,8 +34,10 @@ support-files =
 [test_document_register.html]
 [test_document_register_base_queue.html]
 [test_document_register_lifecycle.html]
+skip-if = true # disabled - See bug 1390396
 [test_document_register_parser.html]
 [test_document_register_stack.html]
+skip-if = true # disabled - See bug 1390396
 [test_document_shared_registry.html]
 [test_event_dispatch.html]
 [test_event_retarget.html]