1845863 - [CtW] HTML placeholder not exposed

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

Reported to me by a JAWS engineer.

STR (with NVDA):

1. Open this test case:
   data:text/html,<input aria-label="label" placeholder="placeholeder">
2. Tab to the input.
   • Expected: NVDA should say "label edit placeholder blank"
   • Actual: NVDA says "label edit blank"

This occurs because we don't include the placeholder attribute in the RemoteAccessible cache. We do include aria-placeholder though, so that does work.

Comment 1

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

Set release status flags based on info from the regressing bug 1824293

Comment 2

[James Teh [:Jamie]]

Attachment: Bug 1845863: Expose HTML placeholder for RemoteAccessible.

We previously cached aria-placeholder, but not HTML placeholder.
These both map to the placeholder object attribute exposed by Accessible::Attributes, but they are retrieved differently internally.

Comment 3

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

Set release status flags based on info from the regressing bug 1824293

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/20ba276f277b
Expose HTML placeholder for RemoteAccessible. r=nlapre

Comment 5

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/20ba276f277b

Comment 6

[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-firefox117 to wontfix.

For more information, please visit BugBot documentation.

Comment 7

[James Teh [:Jamie]]

This patch depends on bug 1743749. That's probably safe enough to uplift, but it's not strictly necessary on beta. Thus, I'm crafting a beta patch which doesn't depend on bug 1743749.

Comment 8

[James Teh [:Jamie]]

Attachment: Bug 1845863: Expose HTML placeholder for RemoteAccessible (beta uplift).

We previously cached aria-placeholder, but not HTML placeholder.
These both map to the placeholder object attribute exposed by Accessible::Attributes, but they are retrieved differently internally.

This also fixes LocalAccessible::Attributes to always prefer HTML placeholder over aria-placeholder as per the ARIA spec.
Previously, the attribute we preferred was somewhat indeterminate (from the user's perspective) due to the ordering of hash maps.

Comment 9

[James Teh [:Jamie]]

Comment on attachment 9347091 [details]
Bug 1845863: Expose HTML placeholder for RemoteAccessible (beta uplift).

Beta/Release Uplift Approval Request

• User impact if declined: HTML placeholder will not be exposed to screen reader users.
• 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): Straightforward patch to include HTML placeholder in the a11y cache. Covered by automated tests.
• String changes made/needed:
• Is Android affected?: No

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9347091 [details]
Bug 1845863: Expose HTML placeholder for RemoteAccessible (beta uplift).

Approved for 117.0b4.

Comment 11

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/ca7b4a682e30

Comment 12

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

Reproduced the issue on Firefox 117.0a1 (2023-07-27) on Windows 10 with NVDA by using the infos provided in Comment 0.

The issue is fixed on Firefox 117.0b4 and Firefox 118.0a1 (2023-08-05) on the same system.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Is this ready for an ESR115 approval request? Though I think it may depend on other patches which aren't currently on that branch.

Comment 14

[James Teh [:Jamie]]

Attachment: Bug 1845863: Expose HTML placeholder for RemoteAccessible (ESR uplift).

We previously cached aria-placeholder, but not HTML placeholder.
These both map to the placeholder object attribute exposed by Accessible::Attributes, but they are retrieved differently internally.

This also fixes LocalAccessible::Attributes to always prefer HTML placeholder over aria-placeholder as per the ARIA spec.
Previously, the attribute we preferred was somewhat indeterminate (from the user's perspective) due to the ordering of hash maps.

Comment 15

[James Teh [:Jamie]]

Comment on attachment 9348963 [details]
Bug 1845863: Expose HTML placeholder for RemoteAccessible (ESR uplift).

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Certain web content information not exposed to screen reader users.
• User impact if declined: HTML placeholder will not be exposed to screen reader users.
• Fix Landed on Version: 117, 118
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Straightforward patch to include HTML placeholder in the a11y cache. Covered by automated tests.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9348963 [details]
Bug 1845863: Expose HTML placeholder for RemoteAccessible (ESR uplift).

Approved for 115.2esr.

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/455c7cac4a43

Comment 18

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

Verified that the issue is fixed on Firefox 115.2.0esr on Windows 10 with NVDA.