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 #1344

10 files changed, 222 insertions, 71 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]
diff --git a/testing/web-platform/meta/custom-elements/CustomElementRegistry.html.ini b/testing/web-platform/meta/custom-elements/CustomElementRegistry.html.ini
index 29be90bea..3f9f66d17 100644
--- a/testing/web-platform/meta/custom-elements/CustomElementRegistry.html.ini
+++ b/testing/web-platform/meta/custom-elements/CustomElementRegistry.html.ini
@@ -6,18 +6,6 @@
   [customElements.define must get "observedAttributes" property on the constructor prototype when "attributeChangedCallback" is present]
     expected: FAIL
 
-  [customElements.define must rethrow an exception thrown while getting observedAttributes on the constructor prototype]
-    expected: FAIL
-
-  [customElements.define must rethrow an exception thrown while converting the value of observedAttributes to sequence<DOMString>]
-    expected: FAIL
-
-  [customElements.define must rethrow an exception thrown while iterating over observedAttributes to sequence<DOMString>]
-    expected: FAIL
-
-  [customElements.define must rethrow an exception thrown while retrieving Symbol.iterator on observedAttributes]
-    expected: FAIL
-
   [customElements.define must upgrade elements in the shadow-including tree order]
     expected: FAIL
 
diff --git a/testing/web-platform/meta/custom-elements/HTMLElement-constructor.html.ini b/testing/web-platform/meta/custom-elements/HTMLElement-constructor.html.ini
new file mode 100644
index 000000000..0d2d4374f
--- /dev/null
+++ b/testing/web-platform/meta/custom-elements/HTMLElement-constructor.html.ini
@@ -0,0 +1,11 @@
+[HTMLElement-constructor.html]
+  type: testharness
+  [HTMLElement constructor must infer the tag name from the element interface]
+    expected: FAIL
+
+  [HTMLElement constructor must allow subclassing a custom element]
+    expected: FAIL
+
+  [HTMLElement constructor must allow subclassing an user-defined subclass of HTMLElement]
+    expected: FAIL
+
diff --git a/testing/web-platform/meta/custom-elements/attribute-changed-callback.html.ini b/testing/web-platform/meta/custom-elements/attribute-changed-callback.html.ini
index 10eea70b2..1b1bea6c9 100644
--- a/testing/web-platform/meta/custom-elements/attribute-changed-callback.html.ini
+++ b/testing/web-platform/meta/custom-elements/attribute-changed-callback.html.ini
@@ -12,21 +12,5 @@
   [setAttributeNode and removeAttributeNS must enqueue and invoke attributeChangedCallback for an SVG attribute]
     expected: FAIL
 
-  [Mutating attributeChangedCallback after calling customElements.define must not affect the callback being invoked]
-    expected: FAIL
-
-  [attributedChangedCallback must not be invoked when the observed attributes does not contain the attribute]
-    expected: FAIL
-
-  [Mutating observedAttributes after calling customElements.define must not affect the set of attributes for which attributedChangedCallback is invoked]
-    expected: FAIL
-
-  [attributedChangedCallback must be enqueued for attributes specified in a non-Array iterable observedAttributes]
-    expected: FAIL
-
   [attributedChangedCallback must be enqueued for style attribute change by mutating inline style declaration]
     expected: FAIL
-
-  [attributedChangedCallback must not be enqueued when mutating inline style declaration if the style attribute is not observed]
-    expected: FAIL
-
diff --git a/testing/web-platform/meta/custom-elements/htmlconstructor/newtarget.html.ini b/testing/web-platform/meta/custom-elements/htmlconstructor/newtarget.html.ini
new file mode 100644
index 000000000..f77a64e1d
--- /dev/null
+++ b/testing/web-platform/meta/custom-elements/htmlconstructor/newtarget.html.ini
@@ -0,0 +1,14 @@
+[newtarget.html]
+  type: testharness
+  [Use NewTarget's prototype, not the one stored at definition time]
+    expected: FAIL
+
+  [Rethrow any exceptions thrown while getting the prototype]
+    expected: FAIL
+
+  [If prototype is not object, derives the fallback from NewTarget's realm (autonomous custom elements)]
+    expected: FAIL
+
+  [If prototype is not object, derives the fallback from NewTarget's realm (customized built-in elements)]
+    expected: FAIL
+