1707022 - "Personalize" drawer does not operate well with a screen reader

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[Eitan Isaacson [:eeejay]]

There are several issues with the personalize drawer in its current implementation. This may be a meta bug, or addressed all directly here.

1. Keyboard activating the "Personalized" button should move keyboard focus to something focusable in the dialog. Arguably the "Close" button since it is the first focusable element in the dialog.
2. The drawer should have a role, probably "dialog" or a landmark role with a title. The title should probably be "personalize". This tells the user when it appears what appeared.
3. The switch/checkbuttons appear semantically before their associated headers. They should follow them.
4. The switch checkboxes are implemented with a known label > checkbox css hack that makes the actual checkbox transparent and small. This makes it a hard touch target for screen readers and doesn't allow users who rely on the reported bounds of the widget to discover it. Ideally, the checkbox should have the same bounds as the visible switch (ie the label).

Comment 1

[Eitan Isaacson [:eeejay]]

In addition to #1, when the dialog is closed focus is not restored in the main page.

Comment 2

[Eitan Isaacson [:eeejay]]

To help prioritize it is worth understanding the user current screen reader user experience:

1. A user tabs or moves the screen reader cursor to the personalize button and activates it.
2. Nothing happens.
3. If the user would tab/cursor forward they will discover that there is now a new "close" button followed by other controls that weren't there before. There is no context to what the close button actually does (does it close the tab?). There is no context to what the following controls do, unless the user remembers that they pressed a button earlier that didn't seem to do anything.
4. The user activates the "Close" button.
5. Nothing happens.
6. The user tabs or cursors forward and now they are inexplicably in the top-sites section, when earlier there were other controls there.

This can be fixed in three steps:

• Add a label "Personalize" and a role of "dialog" to .customize-menu.
• After drawer appears, call focus() on close button.
• After drawer goes away, call focus() on "Personalize" button.

With those fixes, a screen reader user will get all the context they need for the drawer:

1. A user tabs or moves the screen reader cursor to the personalize button and activates it.
2. The user hears "'Personalize' dialog, 'Close' button". They now know they successfully opened a "Personalize" dialog and they are focused on its close button.
3. If the user would tab/cursor forward they will encounter controls with the knowledge that those are customization settings for the new tab.
4. If the user navigates past the last item in the dialog to the top sites section, the screen reader will tell them that they left the dialog.
5. When the user finally navigates to and activates the "Close" button, the screen reader will tell them that the dialog has been closed and they are back where they started on the "Personalize" button.

Comment 3

[Eitan Isaacson [:eeejay]]

tl;dr

The focus management and dialog role are essential for basic screen reader support, the other items can be done in an improvements round.

Comment 4

[Eitan Isaacson [:eeejay]]

I prioritized this as access-s2, and spun out issues #3 and #4 as bug 1708258 and bug 1708259 respectively with an access-s3 severity.

Comment 5

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1707022 - Add dialog role to personalize menu and focus correct buttons when toggled. r?amy

This patch does 3 things:

1. Adds a label "Personalize" and a role of "dialog" to .customize-menu.
2. After drawer appears, calls focus() on close button.
3. When drawer is visible, explicitly hide obscured Personalize button.
4. After drawer goes away, calls focus() on "Personalize" button.

I tried to keep this simple and minimal, but two things made this more
involved:

1. The component heirarchy didn't make it easy to alternate focus
   between two buttons that aren't in the same component.
2. The CSS transition requires waiting for the close button to become
   visible.

I took the liberty to clean up the CSS a bit. There is no need to have
special exceptions for how visibility is animated. Since this is a
binary property it becomes invisible at the end of a transition
duration, and visible at the begining.

Comment 6

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1707022 - p1: Put PersonalizeButton in CustomizeMenu component. r?amy

This will allow us to encapsulate focus control for the menu's open and
close buttons.

Comment 7

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1707022 - p2: Add dialog role and label to personalize menu. r?amy,flod

Depends on D113865

Comment 8

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1707022 - p3: Focus open or close buttons when menu is toggled. r?amy

The timeout for the CSSTransition needs to match the actual duration
of the CSS transition. This is needed because the close button needs to
be visible before focus() is called on it or else it won't recieve
focus.

Depends on D113866

Comment 9

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1707022 - p4: Hide personalize button when menu is showing. r?amy

When the menu is showing the personalize button is obscured, yet
keyboard users can still tab to it. This can be confusing.

I made the transition timeout 50ms longer than the menu transition
because I didn't want it to go away when it was focused before the
close button was focused. This can cause a brief confusion.

Depends on D113867

Comment 10

[Eitan Isaacson [:eeejay]]

Attachment: Bug 1707022 - p5: Clean up menu transition CSS. r?amy

I took the liberty to clean up the CSS a bit. There is no need to have
special exceptions for how visibility is animated. Since this is a
binary property it becomes invisible at the end of a transition
duration, and visible at the begining.

Depends on D113868

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f3d0ce673ad
p1: Put PersonalizeButton in CustomizeMenu component. r=amy

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9b8ce074ff0
p2: Add dialog role and label to personalize menu. r=amy,flod
https://hg.mozilla.org/integration/autoland/rev/a223e40d5d16
p3: Focus open or close buttons when menu is toggled. r=amy
https://hg.mozilla.org/integration/autoland/rev/67a3193e6286
p4: Hide personalize button when menu is showing. r=amy
https://hg.mozilla.org/integration/autoland/rev/285861b32934
p5: Clean up menu transition CSS. r=amy

Comment 13

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/1f3d0ce673ad
https://hg.mozilla.org/mozilla-central/rev/b9b8ce074ff0
https://hg.mozilla.org/mozilla-central/rev/a223e40d5d16
https://hg.mozilla.org/mozilla-central/rev/67a3193e6286
https://hg.mozilla.org/mozilla-central/rev/285861b32934