auro-button: cannot call document.createElement when focus visible polyfill applied to shadow root

[geoffrich]

Describe the bug

Calling document.createElement('auro-button') throws the following exception:

Uncaught DOMException: Failed to construct 'CustomElement': The result must not have attributes

This only happens when the focus-visible polyfill is applied to the shadow root in the button's constructor.

I don't think this has caused any real-world impact. However, it did break tests (#110) and clutters up the console.

To Reproduce

Steps to reproduce the behavior:

1. Bring up the button demo page locally and comment out the if statement in the constructor (or run in a browser that doesn't support focus-visible
2. In the console, call document.createElement('auro-button'). See the error get thrown.

Expected behavior

No exception is thrown

Desktop (please complete the following information):

• OS: Windows 10
• Browser: Chrome
• Version: 90

Additional context

This happens because loading the polyfill in the constructor applies an attribute to the element, and this is a violation of the spec for custom element constructors:

The element must not gain any attributes or children, as this violates the expectations of consumers who use the createElement or createElementNS methods.

I think we need to move the polyfill loading to connectedCallback instead.

[blackfalcon]

@geoffrich I am wondering if this nested support of focus-visible and the polyfill is even worth the trouble? I mean, there is a $30k bounty to get it done.

If I understand this correctly, this is an issue where the button is installed into another CE as a dependency? I am only coming up with examples where the button is content and there are no issues there. The bug you made doesn't reference a CE and the codePen doesn't appear to be working?

But, if we remove this feature from the auro-button, the test issue goes away. Then what do we lose?

Other than Safari, everywhere else this works. When it comes to Safari, the majority of its users are on iOS and that has a completely different experience when it comes to a11y. Then there is the :-webkit-direct-focus. How that will play into things?

And I am coming to the conclusion that this should be closed without merge.

[geoffrich]

I am wondering if this nested support of focus-visible and the polyfill is even worth the trouble? I mean, there is a $30k bounty to get it done.

If I understand this correctly, this is an issue where the button is installed into another CE as a dependency? I am only coming up with examples where the button is content and there are no issues there. The bug you made doesn't reference a CE and the codePen doesn't appear to be working?

Correct, this is only an issue when the button is being used in another CE's shadow DOM. I've fixed the CodePen you linked to show the original issue by pinning to the previous version of the button. This is not a niche use case -- the component hangar uses Auro elements like this in many of its components.

But, if we remove this feature from the auro-button, the test issue goes away. Then what do we lose?

Other than Safari, everywhere else this works.

Currently if we remove this feature from the button, focus styles will break in Safari when the button is inside another shadow DOM.

I am not comfortable excluding Safari users from seeing focus outlines if their browser does not support focus-visible. Sure, desktop Safari has less usage compared to other browsers, but they should still have an accessible experience.

I am more comfortable with the failure case of a guest seeing focus styles when they don't need to see them instead of not seeing focus styles when they need them. The former damages aesthetics, while the latter damages accessibility.

The goal here is for everyone to see focus outlines if they require them. I see two options:

• Continue to apply the polyfill when necessary, including cases where the component is used in other shadow roots.
• Write our CSS to be more resilient, so that all focus styles are on by default and only modified when the browser supports focus-visible. See ":focus-visible and backwards compatibility" from TPG. With this approach, users in browsers that don't support focus-visible will see focus outlines all the time, which I think is better than never seeing them.

I think the latter is where we need to move to deprecate the focus-visible polyfill support as soon as possible. We could do that work as part of this issue and remove the polyfill from auro-button entirely. What do you think?

[blackfalcon]

Sent meeting invite to review options.

[geoffrich]

Meeting outcome: will go ahead with the quick fix of moving the polyfill to connectedCallback and revisit polyfill support at later date. Nesting Auro components is a significant use case due to the component hangar.

[AuroDesignSystem]

🎉 This issue has been resolved in version 6.3.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀