521928 - Other actions button needs to shed focus or stopPropagation or both

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Phil Ringnalda (:philor)]

STR:

1. In a folder with an unread message, select a message other than the unread one, one which is short enough not to have a scrollbar.
2. Click the "other actions" button to open the menu, then click it again to close the menu. At this point, the menu will be closed, and the button will have a focus ring.
3. Press the space bar.

Alternate steps, where we break keyboard nav:

1. In a folder with an unread message, select another message which is not long enough to scroll.
2. Hit F6 (or click in the message body) to focus the message pane, then Shift+Tab to get focus on the other actions button.
3. Press the space bar.

Expected:
The menu opens, nothing else happens.

Actual:
We open the menu (reopen, for the first set of steps), then behind that we navigate to the next unread message (you can tell the order by setting it up so that the header for the first message is taller than the header for the unread message, say by tagging the first one: the popup will then be farther down-screen than it should be, because it was opened relative to where the button was).

For bonus fun (and the reason I want to block on it), when you get to step 3 picture yourself being a blind user with a screen reader: the other actions menu opens, just like you expected, but behind your back we've loaded a new message so my bet would be that you hear nothing, but the menu acts on a message other than the one you thought it would.

The other header buttons avoid similar troubles by calling event.stopPropagation(); RestoreFocusAfterHdrButton(); to avoid the navigation and to avoid hanging onto focus.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Marco, can you verify that the behavior Phil posits is actually what happens when used with a screenreader?

Comment 2

[David Ascher (:davida)]

Cute.  I'm puzzled as how the popup shows anyway on space.  There's no keypress handler, so it must come from the button type="menu" binding.

My guess would be http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/content/widgets/button.xml#244

which should stop propagation.

Only workaround that doesn't touch gecko that I can think of is to put in a specific keypress handler in the button, and stop the lower-level widgetry from working, but that feels a bit scary.

Any other ideas?

Comment 3

[Marco Zehe (:MarcoZ)]

Yes, this happens with screen readers as well.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

blocking it is, then.

Giving to philor, since he seems to have a theory on how to move it forward.  Phil, if you're not available to poke at this, can you give to bwinton?  Thanks!

Comment 5

[Phil Ringnalda (:philor)]

A theory? General, I don't have a theory. All I have is a kind of horrible spasm!

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch to make the "other actions" dropdown work like the "smart reply" dropdown.

So, with this patch, hitting the space bar will now no longer re-open the menu, which perhaps isn't the right thing to do, but is at least consistent with the Smart Reply dropdown.

Marco, could you test this with a screen reader, and let me know how it is?

Thanks,
Blake.

Comment 7

[Marco Zehe (:MarcoZ)]

I don't know about the visual effects of this patch, but with a screen reader, I observe the following:
1. Pressing SPACE or ENTER on the "More Actions" button will not yield any speech. The list drops down, but the first action isn't read. This is no different than without the patch.
2. What is different is that, when closing Escape to close the list again, I do not land back on the button, but I land on the messages list instead. This is definitely not desired behavior. Escape should return focus back to the point where it started from. Observe this in Firefox where, for example, after a list of suggestios pops up in the awesome bar, and you press escape, you land back on the text entry field.

Comment 8

[Marco Zehe (:MarcoZ)]

Comment on attachment 408642 [details] [diff] [review]
A patch to make the "other actions" dropdown work like the "smart reply" dropdown.

r- based on my comment #7 above.

Comment 9

[Blake Winton (:bwinton) (:☕️)]

Attachment: A better patch, I think.

Okay, I think this one should work out a lot better.

I have no idea how to make it say the first item, but perhaps that can be a different patch that goes in after this one?

As a side question, if you use the arrow keys, does it read the items as they're highlight?  If so, perhaps I could set it up to highlight the first item when the popup opens.

Thanks,
Blake.

Comment 10

[Marco Zehe (:MarcoZ)]

(In reply to comment #9)
> As a side question, if you use the arrow keys, does it read the items as
> they're highlight?

Yes.

> If so, perhaps I could set it up to highlight the first
> item when the popup opens.

Don't know if that'll actually go against conventions in regards to this kind of popup button. Let's deal with the one issue first and see how that goes. The other may be a more general issue whose cause lies in the a11y module and not in Thunderbird's Chrome.

Comment 11

[Blake Winton (:bwinton) (:☕️)]

So, I tried running a try-server build.

The output is at:
http://s3.mozillamessaging.com/build/try-server/2009-10-29_08:[e-mail]-1256831193/[e-mail]2

http://s3.mozillamessaging.com/build/try-server/2009-10-29_08:[e-mail]-1256831193/[e-mail]

and

http://s3.mozillamessaging.com/build/try-server/2009-10-29_08:[e-mail]-1256831193/[e-mail]

But it looks on the tinderbox like only the Linux version successfully built, so I'm not sure how helpful that is.

Marco, if you need a specific build, I could try running one in a VM, and shipping it to you somehow.  Just let me know.

Thanks,
Blake.

Comment 12

[Phil Ringnalda (:philor)]

Comment on attachment 409091 [details] [diff] [review]
A better patch, I think.

r=me if you recast either the code or the comment, so that they're both in the same order: the order of the comment matching the order of the code was supposed to explain that "contentWindow.top == window" == "focus is in chrome" and that "document.commandDispatcher.focusedElement && !hRefForClickEvent" means "focus is on a non-link content element like a button" since otherwise neither one of those is exactly obvious.

Oh, and we don't want to worry about the case when a content <a href="..." id="otherActionsButton"> is focused, do we?

Comment 13

[Marco Zehe (:MarcoZ)]

Comment on attachment 409091 [details] [diff] [review]
A better patch, I think.

Confirming that this works with screen readers and focus returns to the proper location when pressing Escape from within the list of Other Actions.

Comment 14

[Blake Winton (:bwinton) (:☕️)]

Nits fixed, and checked in:
http://hg.mozilla.org/releases/comm-1.9.1/rev/8cde8972510d
http://hg.mozilla.org/comm-central/rev/7933b52e249e