Closed Bug 406308 Opened 17 years ago Closed 16 years ago

Don't fire accessible focus events if widget is not actually in focus, confuses screen readers

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: MarcoZ, Assigned: aaronlev)

References

Details

(Keywords: access)

Attachments

(1 file, 6 obsolete files)

To reproduce:
1. Start your favorite screen reader.
2. Launch Thunderbird.
3. In Inbox, set focus to a specific message by arrowing onto it.
4. Shift+Tab to folder list.
5. DownArrow to next folder. Screen reader will speak folder name as expected.
6. UpArrow back to Inbox.

Actual result: The Inbox folder name gets spoken, followed by the announcement that focus has returned to the Message pane and the previously focused message is selected. The screen reader, at this stage, thinks that focus is in the message pane when it actually still is in the Folder pane.

Expected result: Only the Folder name for Inbox and associated information should be spoken.

More information: What is happening is that, once the message pane gets refilled with the information from Inbox, a focus event is issued for the message that was previously selected before leaving the folder. This causes the screen reader to receive focus notification, thinking that focus has automatically shifted to the message pane, and reporting that to the user. The truth is, however, that focus remained in the folder pane and that the blind user has been given incorrect information.

This has always been a problem in Thunderbird, and in TB 3, can be observed on Windows with JAWS or NVDA, and on Linux using Orca.
Flags: blocking-thunderbird3?
Marco, if I get correct we fire focus event on every selection operation on tree, listbox which fires DOMMenuItemActive DOM event and we do not check whether the element is focused actually. And if I get you correct you suggest to fire focus event only when widget is actually focused, right? Can you think of consequences?
Surkov, we should only fire an MSAA focus for DOMMenuItemActive in a tree if the tree has focus.

I don't think any other changes would help address this bug, and would break things anyway.
Attached patch patch (obsolete) — Splinter Review
It seems we always fire focus event because we call FireAccessibleFocusEvent with PR_TRUE in aForceEvent argument (
http://lxr.mozilla.org/mozilla/source/accessible/src/base/nsRootAccessible.cpp#910).
(In reply to comment #3)
> It seems we always fire focus event because we call FireAccessibleFocusEvent
> with PR_TRUE in aForceEvent argument (
> http://lxr.mozilla.org/mozilla/source/accessible/src/base/nsRootAccessible.cpp#910).

This patch does the opposite of what is wanted: It never fires focus events for the widget in focus, it only fires focus events if the widget is not in focus. So with this patch, when arrowing through the Folder List or the Messages List, no focus event gets fired at all.
This bug occures in not just Thunderbird3, but also in Firefox3 (so I would assume any Gecko 1.9 app).
To test in Firefox3:
1. start your favorite screen reader
2. Go to the firefox 3 options dialog, and focus on the 'content' tab list item.
3. Press down arrow to go to the applications tab
Screen readers eventually receive a focus event for the currently selected list item in the applications list, however the focus doesn't actually change physically.
Note this particular example only works when going to the applications tab for the first time in the life of that dialog.
 Using Accessibility probe, it is also clear that these focus events are being fired.
Another example is in the Thunderbird 3 options:
1. Start your favorite screen reader
2. Open the thunderbird 3 options, and focus on the general tab list item
3. press down arrow to move to the display tab.
Focus events are fired on the  html color dropdown button, but again focus doesn't really physically move
Yet another example is in the Thunderbird 3 Address book:
Open the thunderbird 3 address book and make sure you're focused an address book.
Pressing an arrow to move to another address book that contains items, a focus event is fired on the selected item in that address book, yet again, the physical focus did not move. As in, if you press shift+tab, you hit the dialog's  main buttons, not the address book.

So it seems that this bug effects list items, treeview items and drop down buttons so far.

Alexander, are you planning to take a look at this soon? Would be extremely helpful to get this into Firefox 3, since this bites us at every venture into the Add-Ons dialog, some of the Options pages etc. in Firefox, not just Thunderbird, as Mick points out. As I said in comment #4, the patch you put in has the exact opposite effect: It virtually suppresses all events of the widget in focus, but allows the others to bleed through nonetheless.
Attachment #294516 - Attachment is obsolete: true
Attached patch Works well (obsolete) — Splinter Review
Assignee: nobody → aaronleventhal
Attachment #305766 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #305799 - Flags: review?(surkov.alexander)
Attachment #305799 - Flags: review?(ginn.chen)
Ginn or Surkov, if you r+ this please request a? as well so we can possibly get it in before the deadline.
Try builds coming here:
https://build.mozilla.org/tryserver-builds/2008-02-26_16:06-aaronleventhal@moonset.net-CarefulFocusInContainerDescendants/

And if we get a+ maybe someone can check in for me.
(In reply to comment #10)
> Try builds coming here:
> https://build.mozilla.org/tryserver-builds/2008-02-26_16:06-aaronleventhal@moonset.net-CarefulFocusInContainerDescendants/
Major problem: there are now no focus events fired for the menu items along the menu bar. Focus events are fired for menu items with in  each menu, but not along the top (file, edit, view etc).
This is evident in NVDA, but also with Accessibility probe, I do not see the focus events.
However, problems that used to occure in the options dialog (arrowing to the applications page, some widgets in the page would fire focus events even though focus did not change) these are not happening any more.
I can't yet find any other issues than the menu bar one.
I guess we won't fire focus event on xul:menu per Michael's comment because it's not focusable and doesn't implement nsIDOMXULPopupElement, right?

Aaron, why don't you put this code into FireAccessibleFocusEvent where the focus firing logic is?
surkov, the menu has focusable state, although I couldn't focus it by keyboard.

I'm confused by comment #5.
The list item "Content", "Applications" have "focusable" state.
I don't understand "the focus doesn't actually change physically".
Attached patch Fix menu problem (obsolete) — Splinter Review
Makes sense -- I momentarily forgot about special menu fix necessary. In the menu bar the true focus stays in the page and it's also not a popup.
Attachment #305799 - Attachment is obsolete: true
Attachment #305950 - Flags: review?(surkov.alexander)
Attachment #305950 - Flags: review?(ginn.chen)
Attachment #305799 - Flags: review?(surkov.alexander)
Attachment #305799 - Flags: review?(ginn.chen)
Attachment #305950 - Attachment is obsolete: true
Attachment #305951 - Flags: review?(surkov.alexander)
Attachment #305951 - Flags: review?(ginn.chen)
Attachment #305950 - Flags: review?(surkov.alexander)
Attachment #305950 - Flags: review?(ginn.chen)
Attached patch Fix early return (obsolete) — Splinter Review
Attachment #305951 - Attachment is obsolete: true
Attachment #305952 - Flags: review?(surkov.alexander)
Attachment #305952 - Flags: review?(ginn.chen)
Attachment #305951 - Flags: review?(surkov.alexander)
Attachment #305951 - Flags: review?(ginn.chen)
Status: ASSIGNED → NEW
Component: Mail Window Front End → Disability Access APIs
Flags: blocking-thunderbird3?
Product: Thunderbird → Core
QA Contact: front-end → accessibility-apis
Target Milestone: Thunderbird 3 → ---
Attached patch Add |break|Splinter Review
Attachment #305952 - Attachment is obsolete: true
Attachment #305956 - Flags: review?(surkov.alexander)
Attachment #305956 - Flags: review?(ginn.chen)
Attachment #305952 - Flags: review?(surkov.alexander)
Attachment #305952 - Flags: review?(ginn.chen)
Attachment #305956 - Flags: review?(ginn.chen) → review+
Comment on attachment 305956 [details] [diff] [review]
Add |break|

nit: would be nice to move this code into separate method like we have for "TreeInvalidated" and "TreeRowCountChanged" events.
Attachment #305956 - Flags: review?(surkov.alexander)
Attachment #305956 - Flags: review+
Attachment #305956 - Flags: approval1.9?
Attachment #305956 - Flags: approval1.9b4?
Tested with the latest patch, and everything still works as expected.
Comment on attachment 305956 [details] [diff] [review]
Add |break|

a1.9b4=beltzner
Attachment #305956 - Flags: approval1.9b4?
Attachment #305956 - Flags: approval1.9b4+
Attachment #305956 - Flags: approval1.9?
Attachment #305956 - Flags: approval1.9+
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Verified fixed using both version 3.0a1pre (2008022803) and Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b4pre) Gecko/2008022714 Minefield/3.0b4pre
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: