1952571 - Signature that's placed on PDF does not have an accessible name

Status: RESOLVED FIXED

(Firefox :: PDF Viewer, defect, P1)

Description

[Natalie Stroud [:nstroud]]

Attachment: A PDF open in Firefox with NVDA speech viewer open to the bottom right

Attach (recommended) or Link to PDF file here:

Steps to reproduce the problem:

NOTE: You will need a signature saved before completing the steps below

1. Turn on NVDA
2. Using the TAB key, move focus to the 'Add signature' button under the URL bar. Press ENTER
3. Using the TAB key, move focus to a signature button in the doorhanger. Press ENTER
4. The selected signature will appear over the PDF. Observe NVDA output

Expected/Actual:
The signature placed on the PDF should have an accessible name so that screenreader users know their focus is placed on the signature. At this time, NVDA only says 'clickable figure'

Comment 1

[BMO Github Automation]

Attachment: [mozilla/pdf.js] [Editor] Add an aria label to an added signature (bug 1952571) (#19637)

Comment 2

[Monica Chiorean]

Verified on Win11x64 using Firefox build 138.0a1. Now NVDA reads it as 'Signature editor'.
Natalie, can you please confirm this is correct? Thank you.

Comment 3

[Natalie Stroud [:nstroud]]

Hi Monica! I've tested this out and am getting 'Signature editor' with NVDA, thank you! One thing I would note, right after creating a signature and placing focus on the 'signature editor', NVDA will read 'Signature editor [signature name]' (I made 'nat' for a signature and NVDA says : Signature editor nat). If I click away or am adding an already saved signature, the signature editor only seems to say 'Signature editor'. Would it be possible to add the signature name in this case as well? This was screen reader users know which signature is in the signature editor when they place focus?

Thank you again!!

Comment 4

[Monica Chiorean]

@Calixte Denizet this fix does not seem to have reached the Firefox 137.0b9 Beta? Also for the change that Natalie mention in comment#3 can I reopen this issue? Thank you.

Comment 5

[Calixte Denizet (:calixte)]

Yep it's expected to not be fully fixed in beta because it was too late to have the new l10n strings I added in this patch.

Comment 6

[Marco Castelluccio [:marco]]

(In reply to Monica Chiorean from comment #4)

Also for the change that Natalie mention in comment#3 can I reopen this issue? Thank you.

I would open a new bug for that.

Comment 7

[Calixte Denizet (:calixte)]

Sorry, I made a confusion the patch here doesn't have any l10n strings.
Anyway the patch has reached beta:
https://searchfox.org/mozilla-beta/source/toolkit/components/pdfjs/content/build/pdf.mjs#19511
but I forgot to change aria-description for aria-label.

Comment 8

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

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

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

For more information, please visit BugBot documentation.

Comment 9

[Monica Chiorean]

Still reproducing on latest Firefox beta 137.0b10 and release 137.0 also tracking flag mention 137 as affected.
Logged separate bug 1956513 for Natalie's remark.

Comment 10

[Monica Chiorean]

@Marco since 137 was already released and this bug did not make it to 137, can you please update the tracking flag for 137 to won't fix? Thank you.

Comment 11

[Marco Castelluccio [:marco]]

We are planning to uplift it to the 137 dot release.

Comment 12

[Calixte Denizet (:calixte)]

Attachment: Bug 1952571 - Replace aria-description by aria-label for an added signature in pdf.js in 137

Comment 13

[Phabricator Automation]

release Uplift Approval Request

• User impact if declined: Some users won't have the right string when adding a signature with a screen reader
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: Low
• Explanation of risk level: It's a corner case.
• String changes made/needed: No
• Is Android affected?: no

Comment 14

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/16b694b51741