616136 - Give popup notification panels role="alert" and make their close buttons tabbable

Status: VERIFIED FIXED

(Firefox :: Disability Access, defect)

Description

[:Margaret Leibovic]

This is a follow-up to bug 594586. Comments 25-30 talk about this issue.

Bug 594586 comment 25 suggests adding a role="alert" to the panel to solve this problem, but that comment also makes it seem like that decision may require some discussion.

Comment 1

[James Teh [:Jamie]]

Unless I'm missing something, adding role="alert" just informs accessibility APIs that it is an alert that should be read regardless of whether it has focus. The underlying code still needs to be changed to avoid focusing the notification when it appears. Also, f6 needs to be able to  focus the notification, probably its first focusable child.

Comment 2

[:Margaret Leibovic]

(copied over from bug 594586 comment 33)
> questions:
> 
> -do users have any way of knowing that F6 is how you move focus to a
> notification that has role="alert"?
> -how did users focus the old notification bar to interact with it?

I think James's comment above answers Alex's first question. It seems like the main thing we are going to need is a fix that lets the user focus the panel with f6.

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Would it be possible to hang an f6 patch here, and link to a try build? Or is the patch too risky at this point for FF4?

Comment 4

[Marco Zehe (:MarcoZ)]

A friendly ping on this! I think we should make an effort to fix this for FX5. The user interaction model, not just the fact that it grabs focus initially, is a total mess. Focus events aren't fired properly, this thing isn't an alert when it should be, keyboard interaction is unintuitive. For example Escape doesn't return focus back to the document properly.

And judging from feedback I received, users are utterly confused and annoyed by the current doorhanger keyboard interaction model, so this isn't just me or Jamie.

I believe we should turn this bug into one that addresses these problems in one stride, and we get this right. This is an important part of the UI, and it turns out to be more cumbersome than initially anticipated.

Comment 5

[James Teh [:Jamie]]

Here's a list of issues that need fixing with the accessibility of doorhanger notifications. I'm happy to file separate bugs for some of these if required, but I suspect many of them will need to be tackled in one go as Marco suggests.

1. Doorhanger notifications shouldn't receive focus when they appear. (Even putting this objection aside, no focus event is fired in the notification when it appears.)
2. Instead, the user should be able to a) press an accelerator for a button in the notification or b) press f6 to move focus to the notification's first focusable control.
3. The notification "panel" (id: "notification-popup") should have an accessible role of alert (role="alert"?). This will cause it to be reported automatically by screen readers when it appears.
3.1. The notification panel should probably explicitly specify that the label (class: "popup-notification-description") is the description for the panel (using aria-describedby?). This isn't essential, as screen readers generally use algorithms to work this out, but it's more correct.
4. When tabbing through the notification, focus cycles between two buttons: a menu button (class: "popup-notification-menubutton") and a child button (class: "box-inherit button-menubutton-button").
4.1. Both have the same label.
4.2. The menu button reports an accelerator, but not the child button.
4.3. Nothing I do with the menu button seems to activate any sort of menu - at least, not an accessible menu.
4.4. I honestly can't figure out how I'm supposed to interact with this thing at all, except pressing the accelerator.
5. There is a "Close this message" button exposed to accessibility, but you can't tab to it.
6. Pressing escape dismisses the notification, but focus is not returned to the document.

Currently, doorhanger notifications are incredibly unintuitive, obtrusive and awkward for screen reader (and probably other keyboard only) users.

Related NVDA issue ticket: http://www.nvda-project.org/ticket/1438

Comment 6

[Daniel Veditz [:dveditz]]

Not going to block Macaw

Comment 7

[David Bolter [:davidb] (NeedInfo me for attention)]

Marco do you agree with Jamie's detailed summary in comment 5? I'd love to see this all sorted out for FF6 as this is clearly causing user annoyance.

Comment 8

[Marco Zehe (:MarcoZ)]

Yes, I have managed to confirm all of his points from comment 5.

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

Margaret, do you know if we can get a fix onto the FF6 train?

Comment 10

[:Margaret Leibovic]

(In reply to comment #9)
> Margaret, do you know if we can get a fix onto the FF6 train?

I've been busy with other work recently, but this is on my list of things to look at. There are still a number of weeks before we branch to aurora again, so I'll try to work on this some time this week or next.

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

Thanks Margaret!

Removing tracking request, now that I understand that is a request to potentially delay the train!

Comment 12

[:Margaret Leibovic]

(In reply to comment #5)
Jamie, thanks for such a detailed report. I'm sorry it's taken me a little while to look into this, but this is what I've found:

> 1. Doorhanger notifications shouldn't receive focus when they appear. (Even
> putting this objection aside, no focus event is fired in the notification when
> it appears.)
> 2. Instead, the user should be able to a) press an accelerator for a button in
> the notification or b) press f6 to move focus to the notification's first
> focusable control.

This is a problem that came up in bug 594586 comment 28. Adding noautofocus="true" should theoretically stop the panel from taking focus, but it seems that only happens when a chrome element has focus at the time the panel is opened. I'm not familiar with the details of how focus works, but I think it's definitely a platform-dependent issue, and likely a problem I don't know how to solve.

> 3. The notification "panel" (id: "notification-popup") should have an
> accessible role of alert (role="alert"?). This will cause it to be reported
> automatically by screen readers when it appears.
> 3.1. The notification panel should probably explicitly specify that the label
> (class: "popup-notification-description") is the description for the panel
> (using aria-describedby?). This isn't essential, as screen readers generally
> use algorithms to work this out, but it's more correct.

These should be easy to fix in a patch.

> 4. When tabbing through the notification, focus cycles between two buttons: a
> menu button (class: "popup-notification-menubutton") and a child button (class:
> "box-inherit button-menubutton-button").
> 4.1. Both have the same label.

When constructing a <button type="menu-button"> element, you just specify a label for the parent button, and the child button inherits the label as well. It's unfortunate that there are two elements with the same label, when the child button is the one that actually has the command that corresponds to the label.

> 4.2. The menu button reports an accelerator, but not the child button.
> 4.3. Nothing I do with the menu button seems to activate any sort of menu - at
> least, not an accessible menu.

I believe the accelerator corresponds to the action to open the menupopup. When the parent element (class="popup-notification-menubutton") has focus, you should be able to press alt+down (or just down or space on OSX -- I only have a Mac to test with right now) to open the menupopup.

> 4.4. I honestly can't figure out how I'm supposed to interact with this thing
> at all, except pressing the accelerator.

This is probably a problem with <button type="menu-button"> elements in general. I think we should file a follow-up bug to fix this

> 5. There is a "Close this message" button exposed to accessibility, but you
> can't tab to it.

I can fix this in a patch.

> 6. Pressing escape dismisses the notification, but focus is not returned to the
> document.

This seems like another platform-dependent issue.

I'm cc'ing Neil Deakin for these focus/platform issues, but he's away on paternity leave, so I don't expect him to have a chance to look at this for a while. I can make a patch to address the easier issues, but it looks like it will be harder to fix the other issues.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #12)
> When constructing a <button type="menu-button"> element, you just specify a
> label for the parent button, and the child button inherits the label as well.

This sounds like a bug, I think we should file that separately.

Seems like we should fix the tabbability of the close button and the addition of role="alert" in this bug, and then file the other issues (keyboard access of menu-buttons in general, focus issues with panels) as separate bugs.

Comment 14

[:Margaret Leibovic]

Attachment: patch

This patch gives popup notification panels role="alert" and makes their close buttons tabbable. I tried to add an aria-describedby attribute to the panel, but I don't think it's possible to make that work because the popup-notification-description is created dynamically in anonymous content, but the panel itself is defined in browser.xul.

Comment 15

[:Margaret Leibovic]

I'm changing this bug summary to reflect what we're doing here, and I'll file follow-up bugs for the other issues.

Comment 16

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 528687 [details] [diff] [review]
patch

Might be nice to look into switching this back into a normal button (not sure what theming implications that has)...

Comment 17

[:Margaret Leibovic]

Attachment: patch for check-in

(In reply to comment #16)
> Might be nice to look into switching this back into a normal button (not
> sure what theming implications that has)...

Currently we're sharing styles with the .messageCloseButton toolbarbutton, so we would have to figure out how using a button would affect that. We can file a follow-up if it's important, but I'm not sure if it's worth it.

Comment 18

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/5d4aaa69206f

Comment 19

[:Gavin Sharp [email: [e-mail]]]

I didn't realize we were sharing those styles - seems like it's not worth it!

Comment 20

[Neil Deakin]

Were all the issues described in comment 5/12 filed as separate bugs?

Comment 21

[:Margaret Leibovic]

I filed bug 653226 and bug 653230. I think those cover the remaining issues, but I could be wrong.

Comment 22

[AndreiD[QA]]

Verified fixed for Firefox 6 Beta:
 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0) Gecko/20100101 Firefox/6.0

The changes made for the popup notifications panel (described in comment 14, i.e. role="alert") and pushed to m-c (comment 18) are visible in hg, under mozilla beta (i.e. http://hg.mozilla.org/releases/mozilla-beta/file/f3e82fad65b2/browser/base/content/browser.xul)