webcomponents / webcomponents/polyfills

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

Open
#556 1 comment 0 reactions 0 assignees View on GitHub
Focus Area: Standards & Polyfills Type: Bug
Dominant language
HTML
Stars
1.2k
Forks
168
PR merge metrics
No merged PRs in 30d

Description

### Description
According to the [`toggleAttribute(qualifiedName, force)` spec](https://dom.spec.whatwg.org/#dom-element-toggleattribute), 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
```html







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);

```

- 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)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.