[scoped-custom-element-registry] toggleAttribute should only trigger attributeChangedCallback when the attribute is actually toggled

Open jessevanassen opened this issue 2 years ago • 1 comments

Description

According to the toggleAttribute(qualifiedName, force) spec, toggling an attribute should only result in a change if the attribute actually changes. In other words, if the attribute is already present and force is true or if the attribute is not present and force is false, the attribute shouldn't change.

When the polyfill is loaded, the attribute is always changed, resulting in the attributeChangedCallback firing too frequently.

This is also inconsistent with how it works out of the box in the latest versions of Chrome, Firefox and Edge.

It is especially problematic, because it breaks the spec for all web components on the page, not just components that use the scoped registry. In our case, it lead to an unrelated piece of code behaving differently when the polyfill was loaded elsewhere.

Example

<!DOCTYPE html>
<html lang="en">
	<head>
		<meta charset="UTF-8">
		<script src="https://unpkg.com/@webcomponents/[email protected]/src/scoped-custom-element-registry.js"></script>
	</head>
	<body>
		<script type="text/javascript">
			customElements.define('my-element', class MyElement extends HTMLElement {
				static get observedAttributes() {
					return ['param'];
				}

				attributeChangedCallback(name, oldValue, newValue) {
					console.log('attributeChangedCallback', { name, oldValue, newValue });
				}
			});


			const element = document.createElement('my-element');
			document.body.append(element);

			// This should log: first setting of param
			element.setAttribute('param', '');

			// This should log: removing the param
			element.toggleAttribute('param', false);

			// This shouldn't log: parameter is not present
			element.toggleAttribute('param', false);

			// This should log: adding the parameter 
			element.toggleAttribute('param', true);

			// This shouldn't log: parameter is already present
			element.toggleAttribute('param', true);
		</script>
	</body>
</html>

When the polyfill is not included, the attributeChangedCallback fires 3 times, resulting in 3 log statements.
When the polyfill is included, the attributeChangedCallback fires 5 times, resulting in 5 log statements.

Expected behavior

attributeChangedCallback is only called when the attribute is actually toggled

Actual behavior

attributeChangedCallback is also called when the attribute presence remains the same

Version

0.0.9

Browsers affected

[x] Chrome
[x] Firefox
[x] Edge
[ ] Safari
[ ] IE 11

(These are the only browsers I can test)

Aug 22 '23 07:08 jessevanassen

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Apr 26 '25 22:04 stale[bot]