ARIA reflection polyfill contains non-standard props

[nolanlawson]

Some ARIA properties should be reflected between props and attributes, e.g.:

const elm = document.createElement('div')
elm.ariaLabel = 'foo'
elm.getAttribute('aria-label') // 'foo'

We patch these on the global Element, to support the prop-to-attr mapping:

lwc/packages/@lwc/shared/src/aria.ts

Lines 19 to 67 in d93e296

	const AriaPropertyNames = [
	'ariaActiveDescendant',
	'ariaAtomic',
	'ariaAutoComplete',
	'ariaBusy',
	'ariaChecked',
	'ariaColCount',
	'ariaColIndex',
	'ariaColSpan',
	'ariaControls',
	'ariaCurrent',
	'ariaDescribedBy',
	'ariaDetails',
	'ariaDisabled',
	'ariaErrorMessage',
	'ariaExpanded',
	'ariaFlowTo',
	'ariaHasPopup',
	'ariaHidden',
	'ariaInvalid',
	'ariaKeyShortcuts',
	'ariaLabel',
	'ariaLabelledBy',
	'ariaLevel',
	'ariaLive',
	'ariaModal',
	'ariaMultiLine',
	'ariaMultiSelectable',
	'ariaOrientation',
	'ariaOwns',
	'ariaPlaceholder',
	'ariaPosInSet',
	'ariaPressed',
	'ariaReadOnly',
	'ariaRelevant',
	'ariaRequired',
	'ariaRoleDescription',
	'ariaRowCount',
	'ariaRowIndex',
	'ariaRowSpan',
	'ariaSelected',
	'ariaSetSize',
	'ariaSort',
	'ariaValueMax',
	'ariaValueMin',
	'ariaValueNow',
	'ariaValueText',
	'role',
	] as const;

lwc/packages/@lwc/engine-dom/src/polyfills/aria-properties/polyfill.ts

Lines 62 to 69 in 2a9db6d

	export function patch(propName: string) {
	// Typescript is inferring the wrong function type for this particular
	// overloaded method: https://github.com/Microsoft/TypeScript/issues/27972
	// @ts-ignore type-mismatch
	const attrName = AriaPropNameToAttrNameMap[propName];
	const descriptor = createAriaPropertyPropertyDescriptor(propName, attrName);
	Object.defineProperty(Element.prototype, propName, descriptor);
	}

Browsers have been adding support for this reflection, which would allow us to remove the polyfill. (In Firefox it's behind a flag, accessibility.ARIAReflection.enabled.) Unfortunately, not everything we polyfill is actually supported in browsers:

Click to see

Test case	Chrome 101	Safari 15.3	Firefox 99 with flag
ariaActiveDescendant -> aria-activedescendant	❌	❌	❌
ariaAtomic -> aria-atomic	✅	✅	✅
ariaAutoComplete -> aria-autocomplete	✅	✅	✅
ariaBusy -> aria-busy	✅	✅	✅
ariaChecked -> aria-checked	✅	✅	✅
ariaColCount -> aria-colcount	✅	✅	✅
ariaColIndex -> aria-colindex	✅	✅	✅
ariaColSpan -> aria-colspan	✅	✅	✅
ariaControls -> aria-controls	❌	❌	❌
ariaCurrent -> aria-current	✅	✅	✅
ariaDescribedBy -> aria-describedby	❌	❌	❌
ariaDetails -> aria-details	❌	❌	❌
ariaDisabled -> aria-disabled	✅	✅	✅
ariaErrorMessage -> aria-errormessage	❌	❌	❌
ariaExpanded -> aria-expanded	✅	✅	✅
ariaFlowTo -> aria-flowto	❌	❌	❌
ariaHasPopup -> aria-haspopup	✅	✅	✅
ariaHidden -> aria-hidden	✅	✅	✅
ariaInvalid -> aria-invalid	❌	✅	✅
ariaKeyShortcuts -> aria-keyshortcuts	✅	✅	✅
ariaLabel -> aria-label	✅	✅	✅
ariaLabelledBy -> aria-labelledby	❌	❌	❌
ariaLevel -> aria-level	✅	✅	✅
ariaLive -> aria-live	✅	✅	✅
ariaModal -> aria-modal	✅	✅	✅
ariaMultiLine -> aria-multiline	✅	✅	✅
ariaMultiSelectable -> aria-multiselectable	✅	✅	✅
ariaOrientation -> aria-orientation	✅	✅	✅
ariaOwns -> aria-owns	❌	❌	❌
ariaPlaceholder -> aria-placeholder	✅	✅	✅
ariaPosInSet -> aria-posinset	✅	✅	✅
ariaPressed -> aria-pressed	✅	✅	✅
ariaReadOnly -> aria-readonly	✅	✅	✅
ariaRelevant -> aria-relevant	✅	✅	✅
ariaRequired -> aria-required	✅	✅	✅
ariaRoleDescription -> aria-roledescription	✅	✅	✅
ariaRowCount -> aria-rowcount	✅	✅	✅
ariaRowIndex -> aria-rowindex	✅	✅	✅
ariaRowSpan -> aria-rowspan	✅	✅	✅
ariaSelected -> aria-selected	✅	✅	✅
ariaSetSize -> aria-setsize	✅	✅	✅
ariaSort -> aria-sort	✅	✅	✅
ariaValueMax -> aria-valuemax	✅	✅	✅
ariaValueMin -> aria-valuemin	✅	✅	✅
ariaValueNow -> aria-valuenow	✅	✅	✅
ariaValueText -> aria-valuetext	✅	✅	✅
role -> role	✅	✅	✅

In particular, these reflections are not supported:

• ariaActiveDescendant -> aria-activedescendant
• ariaControls -> aria-controls
• ariaDescribedBy -> aria-describedby
• ariaDetails -> aria-details
• ariaErrorMessage -> aria-errormessage
• ariaFlowTo -> aria-flowto
• ariaLabelledBy -> aria-labelledby
• ariaOwns -> aria-owns

We should either 1) remove these reflections from the polyfill, or 2) push browsers/specs to add them.

[uip-robot-zz]

This issue has been linked to a new work item: W-10820032

[nolanlawson]

Hm, just noticed Chrome 101 doesn't support ariaInvalid reflection, whereas Safari does (and so does Firefox with the flag). Might be a Chrome bug.

[nolanlawson]

Filed a bug on Chromium: https://crbug.com/1305421

[nolanlawson]

FWIW I have suggested adding "string" versions of certain props, e.g. ariaControls and ariaActiveDescendant: w3c/aria#1732 (comment)

Admittedly this would solve our (speculative) polyfill problem, but I think there's also a good case regardless of that issue: w3c/aria#1732 (comment)

[nolanlawson]

I have an idea for how we can fix this:

1. Remove the polyfill from the LWC build
2. Move these ARIA props (including non-standard ones) to the LightningElement base class

#2 will work fine for the existing template compiler, because it only uses the property syntax for custom elements, not for built-in elements:

  return [
    api_custom_element("x-cmp", _xCmp, {
      props: {
        ariaActiveDescendant: api_scoped_id("foo"),
        ariaAtomic: "foo",
        ariaAutoComplete: "foo",
        ariaBusy: "foo",
       // ...
      },
      key: 0,
    }),
    api_element("div", {
      attrs: {
        "aria-activedescendant": api_scoped_id("foo"),
        "aria-atomic": "foo",
        "aria-autocomplete": "foo",
        "aria-busy": "foo",
      // ...
      },
      key: 1,
    }),
  ];
}

We will have to apply the standard ones as well, to support Firefox (which still doesn't have ARIA reflection).

On platform, we can still include the @lwc/aria-reflection-polyfill for as long as necessary, to support things like:

const div = document.createElement('div')
div.ariaActiveDescendant = 'foo'

But this will at least allow us to remove the polyfill for off-platform use cases.

[caridy]

That proposal sounds reasonable. Additionally, we could instrument those non-standard accessors installed on LightningElement so we know which one are used... and eventually remove them all, or some if they are not used at all.

[nolanlawson]

Unfortunately it would be tricky to remove the non-standard accessors on LightningElement, since it would also require changing the compiler.

In the short term, I would be most interested in instruming the non-standard accessors on non-LightningElements, since that requires the global polyfills. (And global patches are scary. 😬)

[nolanlawson]

A related problem is that there are some newer ARIA properties that we do not support, either in the polyfill or on LightningElement:

• ariaBrailleLabel
• ariaBrailleRoleDescription
• ariaDescription
• ariaVirtualContent

I would prefer for us to get out of the business entirely of polyfilling these things. But it's not clear to me what we should do in the short term.