Closed
Bug 388187
Opened 17 years ago
Closed 17 years ago
Incorrect initial focus event when dialogs are opened
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
People
(Reporter: jdiggs, Assigned: ginnchen+exoracle)
References
(Blocks 1 open bug)
Details
(Keywords: access, regression, Whiteboard: orca:high)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
aaronlev
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
Steps to reproduce:
1. Launch Accerciser and turn event monitoring on.
2. Launch Thunderbird
3. Get into any of the following dialog boxes:
a. Account wizard
b. Import wizard in address book
c. Page setup dialog
Expected results: Because a radio button has focus, a focus: event and an object:state-changed:focused event would each be issued for the focused radio button.
Actual results: No focus events are issued for the focused radio button.
Notes: This only seems to be the case when the dialog initially appears. If the dialog box loses focus, it will issue the correct events for the focused radio button when it regains focus. Also, when a radio button is the focused item in subsequent steps in the "wizards" (i.e. after you press the Next button) it issues the correct events.
Comment 1•17 years ago
|
||
Joanie, does this occur for all XUL radio buttons -- e.g. in Firefox as well?
Reporter | ||
Comment 2•17 years ago
|
||
It does in the Page Setup dialog -- but in Firefox that is identical to the dialog in Thunderbird, so it's not all that surprising. :-)
I cannot find any other dialogs in Firefox where a radio button has focus as soon as the dialog appears, so it's hard to say. Sorry!
Comment 3•17 years ago
|
||
Note to Mozilla developers:
It's probably because nsRootAccessible::FireCurrentFocusEvent() uses nsAccessNode::GetCurrentFocus(), and it's the radiogroup which Mozilla believes has focus. In the normal case we fire a RadioStateChange in radio.xml's selectedItem setter, and that gets turned into an accessible focus event. In this case, the radiogroup already starts out with that radio item selected, so this method is not called.
I can think of 3 ways of fixing this:
1) Set aaa:activedescendant on radiogroup, but I think that would require an anonid on each radio item
2) In the constructor for radio group search for a selected item and fire RadioStateChange for it. Hopefully that wouldn't cause any regressions with radiogroups that dynamically change the selected item when the radiogroup is first displayed
3) Change FireCurrentFocusEvent(), GetCurrentFocus() or FireAccessibleFocusEvent() to take XUL radiogroups into account, but that seems like a hack
Comment 4•17 years ago
|
||
Actually looks like I'm wrong, the problem is probably elsewhere.
This is broken in Firefox 3 on Windows as well, but it used to work in Firefox 2.
What I'm seeing is:
1) Focus event on radio button
2) Focus event on the HTML document that was previously focused
Joanie, are you seeing that on Linux?
Comment 5•17 years ago
|
||
Joanie, if that's what you're seeing, then the question is why the extra focus event is fired (not why the radio button isn't firing focus events).
Comment 6•17 years ago
|
||
This is fixed in the Firefox page setup dialog as well as the import settings dialog.
Please reopen if you still have issues.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 7•17 years ago
|
||
Oops, I hadn't seen your comments in July. I guess I need to modify my filters to look for my name in bugzilla mail. Sorry!!
I just tried it with the latest trunk and the FF Page Setup dialog (opened via Alt+F, U). When doing so I get a focus: event for the File menu, one for the New Window menu item, and then one for the Document Frame that was previous focused. I am still not getting a focus: event for the Portrait radio button which has focus. I see parallel object:state-changed:focused events (i.e. I get them for the menu, menu item and document frame, but not the radio button).
Reopening.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment 8•17 years ago
|
||
Hmm, I'm not seeing this bug on Windows anymore as I used to, either with a debug or a standard release build. I'll have to look on Linux when I get to the office next week.
Updated•17 years ago
|
Depends on: fox3access
Updated•17 years ago
|
Blocks: fox3access
No longer depends on: fox3access
Comment 9•17 years ago
|
||
I think this is broken (on Linux only) for the initial focus in any dialog.
If I increase the timeout to 1000 for FireFocusCallback() in nsRootAccessible then the bug is fixed on my system.
But, that is arbirtrary, I'd like to base things off of a better notification when the dialog is really ready.
Comment 10•17 years ago
|
||
Anyway, we fire that timeout whenever a new root accessible is created because a new window is being created. There's a timeout (normally length 0) because focus isn't ready yet. Anyway, the timeout isn't working.
Comment 11•17 years ago
|
||
Might want to try and do the
mFireFocusTimer->InitWithFuncCallback() in a "pageshow" handler in nsRootAccessible. But, only for root accessibles that are shown.
Assignee: aaronleventhal → ginn.chen
Status: REOPENED → NEW
Assignee | ||
Comment 12•17 years ago
|
||
Yes, the focus event is fired before the dialog window is displayed.
comment #11 sounds good. I'll try.
Assignee | ||
Comment 13•17 years ago
|
||
Aaron, is it correct?
Does it work on Windows?
On Linux, activate event is right after focus event for nsWindow.
See nsWindow::SetFocus()
BTW:
I didn't do much testing with this patch yet, and I may not have a chance to do it in next week.
Attachment #282819 -
Flags: review?(aaronleventhal)
Comment 14•17 years ago
|
||
It's a good idea Ginn.
Marco, can you test with this patch and see if it breaks anything on Windows?
Comment 15•17 years ago
|
||
Ginn, this was a good idea. Since you are not available as much next week, I hope you don't mind that I've posted what I think is an improvement.
Three changes:
1) Move to another place in code that already deals with focus returning to the root accessible, so that it can all be in one place
2) More specific comments
3) Don't recreate mFireFocusTimer if it already exists (this happens for example if you Alt+Tab to another window and Alt+Tab back)
Attachment #282819 -
Attachment is obsolete: true
Attachment #282867 -
Flags: review?(ginn.chen)
Attachment #282819 -
Flags: review?(aaronleventhal)
Comment 16•17 years ago
|
||
Marco, please test with the new patch instead, and if you don't mind please test it on Linux as well to see if it still fixes the original bug.
Comment 17•17 years ago
|
||
(In reply to comment #16)
> Marco, please test with the new patch instead, and if you don't mind please
> test it on Linux as well to see if it still fixes the original bug.
Unfortunately, it doesn't fix the original bug on Linux. On Windows, I don't notice any difference in behaviour. While Ginn's patch fixes the bug on Linux, yours doesn't.
Assignee | ||
Comment 18•17 years ago
|
||
Aaron, your patch looks good.
But I need to debug into it to figure out why it doesn't work.
I'll find some time to do that tomorrow.
Assignee | ||
Comment 19•17 years ago
|
||
Comment on attachment 282867 [details] [diff] [review]
Ginn's fix, with some tweaks
GetCurrentFocus() doesn't work here.
It doesn't give us what we want.
In nsRootAccessible::FireAccessibleFocusEvent(),
we can use (aNode == mDOMNode), or we can move the code back to nsRootAccessible::HandleEventWithTarget
Attachment #282867 -
Flags: review?(ginn.chen) → review-
Assignee | ||
Comment 20•17 years ago
|
||
Attachment #282867 -
Attachment is obsolete: true
Attachment #283162 -
Flags: review?(aaronleventhal)
Updated•17 years ago
|
Attachment #283162 -
Flags: review?(aaronleventhal) → review+
Updated•17 years ago
|
Attachment #283162 -
Flags: approval1.9?
Updated•17 years ago
|
Summary: Focused radio button should issue focus events when dialogs appear → Incorrect initial focus event when dialogs are opened
Updated•17 years ago
|
Severity: normal → major
Whiteboard: orca:high
Updated•17 years ago
|
Attachment #283162 -
Flags: approval1.9? → approval1.9+
Comment 21•17 years ago
|
||
Ginn, we seem to think this is fixed? Should it be marked RESOLVED FIXED?
Comment 22•17 years ago
|
||
(In reply to comment #0)
> Steps to reproduce:
>
> 1. Launch Accerciser and turn event monitoring on.
> 2. Launch Thunderbird
> 3. Get into any of the following dialog boxes:
> a. Account wizard
> b. Import wizard in address book
> c. Page setup dialog
>
> Expected results: Because a radio button has focus, a focus: event and an
> object:state-changed:focused event would each be issued for the focused radio
> button.
>
> Actual results: No focus events are issued for the focused radio button.
>
> Notes: This only seems to be the case when the dialog initially appears. If
> the dialog box loses focus, it will issue the correct events for the focused
> radio button when it regains focus. Also, when a radio button is the focused
> item in subsequent steps in the "wizards" (i.e. after you press the Next
> button) it issues the correct events.
>
Status: NEW → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Comment 23•17 years ago
|
||
Fix checked in, sorry for the extra noise.
You need to log in
before you can comment on or make changes to this bug.
Description
•