From 794d56d6276258f39f09108c5f2c9451609e0b94 Mon Sep 17 00:00:00 2001
From: Steve Orvell <steve@orvellia.com>
Date: Thu, 20 Jun 2024 12:53:40 -0700
Subject: [PATCH] [scoped-custom-element-registry] Fix attributeChangedCallback
 calling for parser crated elements and toggleAttribute (#583)

- parser created custom elements call attributeChangedCallback for parser created attributes
- toggleAttribute called only when attribute value changes
---
 .../CHANGELOG.md                              |  7 ++
 .../src/scoped-custom-element-registry.ts     | 82 ++++++++++++++++---
 .../test/Element.test.html                    | 21 +++++
 .../test/Element.test.html.js                 | 30 ++++++-
 .../test/utils.js                             |  4 +
 5 files changed, 129 insertions(+), 15 deletions(-)

diff --git a/packages/scoped-custom-element-registry/CHANGELOG.md b/packages/scoped-custom-element-registry/CHANGELOG.md
index c45cefe89..8c1b4edfe 100644
--- a/packages/scoped-custom-element-registry/CHANGELOG.md
+++ b/packages/scoped-custom-element-registry/CHANGELOG.md
@@ -23,6 +23,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - formAssociated set by first name's defining value or if
   CustomElementRegistryPolyfill.formAssociated set contains name
 
+### Fixed
+
+- parser created custom elements call attributeChangedCallback for parser
+  created attributes
+
+- toggleAttribute called only when attribute value changes
+
 ## [0.0.9] - 2023-03-30
 
 - Update dependencies ([#542](https://github.com/webcomponents/polyfills/pull/542))
diff --git a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts
index 0885f299d..b68062d27 100644
--- a/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts
+++ b/packages/scoped-custom-element-registry/src/scoped-custom-element-registry.ts
@@ -416,6 +416,7 @@ const createStandInElement = (tagName: string): CustomElementConstructor => {
       this: HTMLElement,
       ...args: ParametersOf<CustomHTMLElement['connectedCallback']>
     ) {
+      ensureAttributesCustomized(this);
       const definition = definitionForElement.get(this);
       if (definition) {
         // Delegate out to user callback
@@ -514,6 +515,7 @@ const patchAttributes = (
   const setAttribute = elementClass.prototype.setAttribute;
   if (setAttribute) {
     elementClass.prototype.setAttribute = function (n: string, value: string) {
+      ensureAttributesCustomized(this);
       const name = n.toLowerCase();
       if (observedAttributes.has(name)) {
         const old = this.getAttribute(name);
@@ -527,6 +529,7 @@ const patchAttributes = (
   const removeAttribute = elementClass.prototype.removeAttribute;
   if (removeAttribute) {
     elementClass.prototype.removeAttribute = function (n: string) {
+      ensureAttributesCustomized(this);
       const name = n.toLowerCase();
       if (observedAttributes.has(name)) {
         const old = this.getAttribute(name);
@@ -543,12 +546,15 @@ const patchAttributes = (
       n: string,
       force?: boolean
     ) {
+      ensureAttributesCustomized(this);
       const name = n.toLowerCase();
       if (observedAttributes.has(name)) {
         const old = this.getAttribute(name);
         toggleAttribute.call(this, name, force);
         const newValue = this.getAttribute(name);
-        attributeChangedCallback.call(this, name, old, newValue);
+        if (old !== newValue) {
+          attributeChangedCallback.call(this, name, old, newValue);
+        }
       } else {
         toggleAttribute.call(this, name, force);
       }
@@ -556,6 +562,58 @@ const patchAttributes = (
   }
 };
 
+// Helper to defer initial attribute processing for parser generated
+// custom elements. These elements are created without attributes
+// so attributes cannot be processed in the constructor. Instead,
+// these elements are customized at the first opportunity:
+// 1. when the element is connected
+// 2. when any attribute API is first used
+// 3. when the document becomes readyState === interactive (the parser is done)
+let elementsPendingAttributes: Set<CustomHTMLElement & HTMLElement> | undefined;
+if (document.readyState === 'loading') {
+  elementsPendingAttributes = new Set();
+  document.addEventListener(
+    'readystatechange',
+    () => {
+      elementsPendingAttributes!.forEach((instance) =>
+        customizeAttributes(instance, definitionForElement.get(instance)!)
+      );
+    },
+    {once: true}
+  );
+}
+
+const ensureAttributesCustomized = (
+  instance: CustomHTMLElement & HTMLElement
+) => {
+  if (!elementsPendingAttributes?.has(instance)) {
+    return;
+  }
+  customizeAttributes(instance, definitionForElement.get(instance)!);
+};
+
+// Approximate observedAttributes from the user class, since the stand-in element had none
+const customizeAttributes = (
+  instance: CustomHTMLElement & HTMLElement,
+  definition: CustomElementDefinition
+) => {
+  elementsPendingAttributes?.delete(instance);
+  if (!definition.attributeChangedCallback) {
+    return;
+  }
+  definition.observedAttributes.forEach((attr: string) => {
+    if (!instance.hasAttribute(attr)) {
+      return;
+    }
+    definition.attributeChangedCallback!.call(
+      instance,
+      attr,
+      null,
+      instance.getAttribute(attr)
+    );
+  });
+};
+
 // Helper to patch CE class hierarchy changing those CE classes created before applying the polyfill
 // to make them work with the new patched CustomElementsRegistry
 const patchHTMLElement = (elementClass: CustomElementConstructor): unknown => {
@@ -587,17 +645,17 @@ const customize = (
     new definition.elementClass();
   }
   if (definition.attributeChangedCallback) {
-    // Approximate observedAttributes from the user class, since the stand-in element had none
-    definition.observedAttributes.forEach((attr) => {
-      if (instance.hasAttribute(attr)) {
-        definition.attributeChangedCallback!.call(
-          instance,
-          attr,
-          null,
-          instance.getAttribute(attr)
-        );
-      }
-    });
+    // Note, these checks determine if the element is being parser created.
+    // and has no attributes when created. In this case, it may have attributes
+    // in HTML that are immediately processed. To handle this, the instance
+    // is added to a set and its attributes are customized at first
+    // opportunity (e.g. when connected or when the parser completes and the
+    // document becomes interactive).
+    if (elementsPendingAttributes !== undefined && !instance.hasAttributes()) {
+      elementsPendingAttributes.add(instance);
+    } else {
+      customizeAttributes(instance, definition);
+    }
   }
   if (isUpgrade && definition.connectedCallback && instance.isConnected) {
     definition.connectedCallback.call(instance);
diff --git a/packages/scoped-custom-element-registry/test/Element.test.html b/packages/scoped-custom-element-registry/test/Element.test.html
index 36db1e8ca..0d48c6a98 100644
--- a/packages/scoped-custom-element-registry/test/Element.test.html
+++ b/packages/scoped-custom-element-registry/test/Element.test.html
@@ -1,6 +1,27 @@
 <html>
   <body>
     <script src="../scoped-custom-element-registry.min.js"></script>
+
+    <script>
+      // Test element for testing attribute processing of parser created
+      // elements.
+      customElements.define(
+        'parsed-el',
+        class extends HTMLElement {
+          static observedAttributes = ['a', 'b'];
+          attributeChanges = [];
+          attributeChangedCallback(name, old, value) {
+            this.attributeChanges.push({name, old, value});
+          }
+        }
+      );
+      const imp = document.createElement('parsed-el');
+      imp.setAttribute('a', 'ia');
+      imp.id = 'imperative-parsed-el';
+      document.body.append(imp);
+    </script>
+    <parsed-el id="parsed-el" a="a" b="b"></parsed-el>
+
     <script type="module">
       import {runTests} from '@web/test-runner-mocha';
 
diff --git a/packages/scoped-custom-element-registry/test/Element.test.html.js b/packages/scoped-custom-element-registry/test/Element.test.html.js
index 09582fb2c..4d3df101b 100644
--- a/packages/scoped-custom-element-registry/test/Element.test.html.js
+++ b/packages/scoped-custom-element-registry/test/Element.test.html.js
@@ -119,7 +119,9 @@ describe('Element', () => {
       const $el = document.createElement(tagName);
 
       $el.setAttribute('foo', 'bar');
-
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'foo', old: null, value: 'bar'},
+      ]);
       expect($el.getAttribute('foo')).to.equal('bar');
     });
 
@@ -131,7 +133,10 @@ describe('Element', () => {
       const $el = getHTML(`<${tagName} foo></${tagName}>`);
 
       $el.removeAttribute('foo');
-
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'foo', old: null, value: ''},
+        {name: 'foo', old: '', value: null},
+      ]);
       expect($el.hasAttribute('foo')).to.be.false;
     });
 
@@ -148,8 +153,27 @@ describe('Element', () => {
 
       $el.setAttribute('foo', '');
       $el.toggleAttribute('foo', true);
-
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'foo', old: null, value: ''},
+      ]);
       expect($el.hasAttribute('foo')).to.be.true;
     });
+
+    it('should call attributeChangedCallback for parser created element', () => {
+      const $el = document.getElementById('parsed-el');
+      expect($el).to.be.ok;
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'a', old: null, value: 'a'},
+        {name: 'b', old: null, value: 'b'},
+      ]);
+    });
+
+    it('should call attributeChangedCallback for imperative created element while parsing', () => {
+      const $el = document.getElementById('imperative-parsed-el');
+      expect($el).to.be.ok;
+      expect($el.attributeChanges).to.be.deep.equal([
+        {name: 'a', old: null, value: 'ia'},
+      ]);
+    });
   });
 });
diff --git a/packages/scoped-custom-element-registry/test/utils.js b/packages/scoped-custom-element-registry/test/utils.js
index 409d19b85..809612a43 100644
--- a/packages/scoped-custom-element-registry/test/utils.js
+++ b/packages/scoped-custom-element-registry/test/utils.js
@@ -43,6 +43,10 @@ export const getObservedAttributesTestElement = (
     static get observedAttributes() {
       return observedAttributeNames;
     }
+    attributeChanges = [];
+    attributeChangedCallback(name, old, value) {
+      this.attributeChanges.push({name, old, value});
+    }
   },
 });