1853209 - ARIA reflection should treat setting null/undefined as removing the attribute

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Nolan Lawson]

Attachment: repro.html

Steps to reproduce:

1. Set the ariaLabel on an element to the string "foo"
2. Set the ariaLabel to null or undefined

Here is a minimal repro:

const div = document.createElement('div')

div.ariaLabel = 'foo'
div.ariaLabel = null
console.log(div.outerHTML)

div.ariaLabel = 'foo'
div.ariaLabel = undefined
console.log(div.outerHTML)

And a CodePen to demonstrate: https://codepen.io/nolanlawson-the-selector/pen/OJrjPoq

Actual results:

Firefox sets the value of the attribute as the literal string "null" or "undefined":

<div aria-label="null"></div>
<div aria-label="undefined"></div>

Expected results:

Aligning with Chrome and Safari's behavior, it should remove the attribute:

<div></div>
<div></div>

Note that I am using Firefox Nightly 119.0a1 (2023-09-14) (64-bit). My goal is to test the fix for this issue: https://bugzilla.mozilla.org/show_bug.cgi?id=1785412

Also note that there is some disagreement in the spec as to whether null should be treated the same as undefined: https://github.com/w3c/aria/issues/1858

However, if your goal is webcompat with Chrome and Safari, then you should treat both null and undefined as removing the attribute.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Disability Access APIs' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Eitan Isaacson [:eeejay]]

spec disagreement aside these definitely should not be made into strings. Setting severity accordingly.

Comment 3

[James Teh [:Jamie]]

We should:

1. Use DOMString? instead of DOMString for the attributes in dom/webidl/AriaAttributes.webidl.
2. Change the setter for REFLECT_DOMSTRING_ATTR to use SetOrRemoveNullableStringAttr instead of SetAttr.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug has a release status flag that shows some version of Firefox is affected, thus it will be considered confirmed.

Comment 5

[Nolan Lawson]

Possibly related issue: the value returned by the getter when the attribute is not set is the empty string rather than null:

const div = document.createElement('div')
console.log(div.role) // empty string, should be null
console.log(div.ariaLabel) // empty string, should be null

In Chrome/Safari, this will return null, but in Firefox Nightly 20.0a1 (2023-10-11) it's the empty string: https://codepen.io/nolanlawson-the-selector/pen/yLGwjwK

The spec also specifies that it should return null. https://w3c.github.io/aria/#idl-reflection-attribute-values

On getting, a missing ARIA attribute will return null.

Comment 6

[James Teh [:Jamie]]

This will effectively revert bug 1647296. Apparently, these attributes were nullable initially, then weren't nullable and are now nullable again. :)

Comment 7

[James Teh [:Jamie]]

Attachment: Bug 1853209: Remove the content attribute when a JS ARIA attribute is set to null.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/31b210944701
Remove the content attribute when a JS ARIA attribute is set to null. r=edgar

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42550 for changes under testing/web-platform/tests

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/31b210944701

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:Jamie, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox119 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[James Teh [:Jamie]]

Comment on attachment 9357984 [details]
Bug 1853209: Remove the content attribute when a JS ARIA attribute is set to null.

Beta/Release Uplift Approval Request

• User impact if declined: Parts of DOM ARIA string attribute reflection don't work as specified, which might break websites using this feature.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only impacts ARIA reflection, which is a new web feature shipping in 119. Covered by automated tests.
• String changes made/needed:
• Is Android affected?: Yes

Comment 13

[Donal Meehan [:dmeehan]]

Comment on attachment 9357984 [details]
Bug 1853209: Remove the content attribute when a JS ARIA attribute is set to null.

119.0 is now in RC, changing to a release uplift request

Comment 14

[Donal Meehan [:dmeehan]]

Aiming to include this in an 119.0 RC2

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 17

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 18

[Donal Meehan [:dmeehan]]

Comment on attachment 9357984 [details]
Bug 1853209: Remove the content attribute when a JS ARIA attribute is set to null.

Approved for 119.0 RC2

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/c2216bb59977

Comment 20

[Catalin Sasca, Desktop Test Engineering [:csasca]]

Reproduced the issue on Firefox 119.0a1 (2023-09-14) under macOS 13.6 by following the infos provided in Comment 0.

The issue is fixed on Firefox 119.0 RC2 and Firefox 120.0a1 (2023-10-19). Tests were performed on macOS 13.6, Windows 11 and Ubuntu 22.04.