1997388 - [dialog] New Shortcut dialog needs a label/accessible name

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Natalie Stroud [:nstroud]]

Attachment: Homepage new tab open in Nightly with the New Shortcut modal open and highlighted by dev tools. Dev tools shows the dialog is not labeled

STR:

1. Open Firefox Nightly
2. Go to about:newtab
3. In the shortcuts row, click the 'Add shortcut' button
4. The 'New Shortcut' dialog will appear. Open 'Web Developer Tools' (press Ctrl + Shift + I or open the hamburger menu at the top right, More tools > Web Developer Tools)
5. In dev tools, click the Accessibility tab.
6. Click the 'Pick accessible object from page' icon button to the left of 'Inspector'.
7. Hover over the entire dialog. Once it's highlighted, click on the dialog.

Actual / Expected:

NVDA only says 'dialog' when the New Shortcuts dialog appears. Labeling the dialog allows assistive technology users to know what its purpose is. See: Dialogs should be labeled

Comment 1

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:thecount, could you have a look please?

For more information, please visit BugBot documentation.

Comment 2

[Reem Hamoui]

To help Mozilla out with this bug, here's the steps:

1. Comment here on the bug that you want to volunteer to help.
   This will tell others that you're working on the next steps.

2. Download and build the Firefox source code: https://firefox-source-docs.mozilla.org/setup/index.html

   • If you have any problems, please ask on the bug or reach out to the team.
   • You can also read the Firefox Contributors' Quick Reference, which has answers to most development questions: https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html

3. Start working on this bug. Follow the steps the reporter of this bug has put in to replicate the issue.

   • If you have any problems with this bug, please comment on this bug and set the needinfo flag for me.

4. Run and test your newtab changes:

   • Open 2 terminal windows, both in the firefox directory:
     • Window #1: Run ./mach newtab watch (this will watch for code changes)
     • Window #2: Run ./mach run (this will launch Firefox)
   • To reload the page with fresh code changes:
     • Mac: Press Cmd+Opt+R
     • Windows/Linux: Press Ctrl+Shift+R
   • Check your changes for adherence to our style guidelines by using ./mach lint

5. Commit your changes with a proper commit message:

   • Use the following format for your commit message:
     Bug XXXXXX - Text explaining your fix. r=#home-newtab-reviewers
     • Replace XXXXXX with the bug number (found at the end of this bug's URL)
     • Example: Bug 1234567 - Fix broken link in Top Sites menu. r=#home-newtab-reviewers
   • Commit your changes with: git commit -am "Bug XXXXXX - Your description here. r=#home-newtab-reviewers"
   • Important: Phabricator only needs one commit per bug. If you need to make additional changes after committing, use git commit --amend --no-edit to update your existing commit instead of creating a new one.

6. Submit the patch (including an automated test, if applicable) for review. Mark me as a reviewer so I'll get an email to come look at your code.

   • Getting your code reviewed: https://firefox-source-docs.mozilla.org/setup/contributing_code.html#getting-your-code-reviewed
   • This is when the bug will be assigned to you.

7. After a series of reviews and changes to your patch, I'll mark it for checkin or push it to autoland. Your code will soon be shipping to Firefox users worldwide!

8. ...now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution.

Comment 3

[karan68]

Can someone assign me this bug!

Comment 4

[karan68]

@[e-mail] please assign me this

Comment 5

[karan68]

Attachment: Bug 1997388 - Label the New Shortcut dialog for accessibility. r=#home-newtab-reviewers

The <dialog> element in ModalOverlayWrapper had no accessible name, causing
screen readers to announce only 'dialog'. Moved aria-labelledby from the inner
div to the <dialog> element, added an id to the TopSiteForm heading, and passed
headerId to ModalOverlayWrapper so the dialog is labeled 'New Shortcut'.

Comment 6

[Reem Hamoui]

Hey there, sorry I was not getting notified. With Bugzilla, you'll need to go to the bottom of the page (under the textbox) and select "Request information from" and put the username in its input.

Happy to assign this bug to you

Comment 7

[karan68]

Also I have raised the PR for this - https://phabricator.services.mozilla.com/D282220 can you please check and approve it , if it is ok

Comment 8

[Reem Hamoui]

Hey there, thanks for being patient! I've just reviewed the patch for this bug.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/049f5f7899b9
https://hg.mozilla.org/integration/autoland/rev/13c4d63388fd
Label the New Shortcut dialog for accessibility. r=home-newtab-reviewers,reemhamz

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ea33608d0d2e
https://hg.mozilla.org/integration/autoland/rev/3e11b69f69b3
Revert "Bug 1997388 - Label the New Shortcut dialog for accessibility. r=home-newtab-reviewers,reemhamz" for causing newtab failures @bundles

Comment 11

[pstanciu]

Backed out for causing newtab failures @bundles

• https://hg.mozilla.org/integration/autoland/rev/3e11b69f69b3eb59ea1ca440c2f4d4d309de279b
• Push with failures
• Failure Log

Comment 12

[Reem Hamoui]

Hey there, just seeing if you'd still be keen to finish working on the patch?
I've added instructions on the patch for how to fix the errors.

Comment 13

[BugBot [:suhaib / :marco]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:thecount, since the bug has recent activity, could you have a look please?

For more information, please visit BugBot documentation.

Comment 14

[liz]

can this be reassigned to me?

Comment 15

[Kelly Cochrane [:kcochrane]]

Going to defer to Reem on how to proceed here. It looks like there was already a decent amount of work done in the previous patch, so we may just prefer to commandeer it and fix the test failures rather than abandoning that and starting from scratch with a new contributor.

Comment 16

[Reem Hamoui]

Hey liz, I'm going to go ahead and commandeer this patch as Kelly suggested, as it's been lingering in limbo for a while.
Sorry about that -- let me know if there are any other new tab bugs you'd be interested in working on!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1125ae0a3a30
https://hg.mozilla.org/integration/autoland/rev/c0d72bf78ed3
Label the New Shortcut dialog for accessibility. r=home-newtab-reviewers,ini

Comment 18

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/c0d72bf78ed3

Comment 19

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/c0d72bf78ed3