580464 - Accessible focus incorrect after JS focus() but correct after switching apps or using menu bar

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

User-Agent:       Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; en-US; rv:2.0b2pre) Gecko/20100712 Minefield/4.0b2pre
Build Identifier: Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; en-US; rv:2.0b2pre) Gecko/20100712 Minefield/4.0b2pre

On the page specified in the URL for this bug, alertdialog #2 and #3 set focus to the dialog and to a button inside the dialog, respectively. For some reason, in both cases, the focus according to accessibility is set to the parent of the dialog. However, after switching applications or using the menu bar, the focus is corrected.

Reproducible: Always

Steps to Reproduce:
1. Open the URL specified in the bug.
2. Press the "Open alertdialog #2" button.
Expected: The focus according to accessibility should be on the alertdialog.
Actual: The focus according to accessibility is on the parent of the alertdialog (class: alertdialogtest). This includes both the focus event and the focused state.
3. Activate the menu bar by pressing alt.
4. Deactivate the menu bar by pressing alt.
Observation: The focus is now on the alertdialog as expected, including the focused state.
5. Press the Close button inside the alertdialog.

6. Press the "Open alertdialog #3" button.
Expected: The focus according to accessibility should be on the Close button inside the alertdialog.
Actual: The focus according to accessibility is on the parent of the alertdialog (class: alertdialogtest). This includes both the focus event and the focused state.
7. Activate the menu bar by pressing alt.
8. Deactivate the menu bar by pressing alt.
Observation: The focus is now on the Close button inside the alertdialog as expected, including the focused state.



I have no idea what is causing this. I haven't figured out how to reproduce it using my own code. It's certainly not just the use of .focus() on DOM nodes.

Works correctly in Firefox 3.6.x. I'm pretty sure this also used to work in Firefox 3.7pre about two months ago, but I need to verify this.

Comment 1

[James Teh [:Jamie]]

Not sure if this is related, but I've just noticed that the "Check for Updates" dialog exhibits similarly strange behaviour when focusing the OK button when there are no updates. The focus according to accessibility is on a property page (xul:hbox), including the focused state. However, switching applications and then returning corrects the focus so that it is on the OK button. Again, this is a regression from 3.6.x.

Comment 2

[Marco Zehe (:MarcoZ)]

I'm seeing this, too. This used to work in the July 3, 2010 build of Minefield, but is broken in the July 19, 2010 build, so the change happened somewhere between those two dates.

Comment 3

[Marco Zehe (:MarcoZ)]

Regression window: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=580205b95ceb&tochange=82edf5bd1abe Implicates bug 574312. Works in the June 30, 2010 build, broken in the July 1, 2010 build.

Comment 4

[alexander :surkov (:asurkov)]

requesting blocking status based on comment #3.

Comment 5

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

It looks like GetAccessibleByRule (formerly GetRelevantContentNodeFor) relies on the cache more. Possibly this is timing related.

Comment 6

[Marco Zehe (:MarcoZ)]

It is definitely 100% reproducible in the Help/Check for Updates dialog, regardless of whether Minefield finds an update and displays the info and the "Install and restart" button, or if it finds nothing and just displays the simple dialog. Focus is always claimed to be on the tab panel accessible rather than the actual button now.

Comment 7

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

As a data point, in step 3 of comment 0, I get the following events (among others):
EVENT_OBJECT_FOCUS name = null role = section state = [focused, defunct] 
Followed 15 msecs later by:
EVENT_OBJECT_SHOW name = Blah blah role = dialog state = [focusable]

So it seems we are getting a focus event on something that isn't ready for prime time. I suspect we are handling the focus event too quickly (in our gecko a11y code) now. My hunch is the change to nsRootAccessible::FireCurrentFocusEvent in rev 51b69f8d306f...

Comment 8

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

Trying the Help/Check for Updates STR, and hitting refresh on accprobe. I no longer see the root frame for FF, but instead a top level Software Update dialog. There are 18 property pages only one of which is not invisible, offscreen which seems correctly to be the No Updates Found one.

I think what may be happening here is that first a property page "Checking for Updates" is visible, then goes invisible, and "No Updates Found" becomes visible. I suspect the timing of this is causing problems.

Comment 9

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

Assigning to myself for now.

Comment 10

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

Note this may have the same root cause as bug 580535 and so might better belong to Alexander, but I hope to spend some time here soon.

Comment 11

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

Attachment: simplified testcase

Comment 12

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

It is definitely timing. If we are notified of focus too quickly after the div is displayed, the accessible is not in the cache, and we walk up the parent chain looking for an accessible to tie to the event. I confirmed that adding some time in between the display and the focus makes the focus event correct.

Comment 13

[Marco Zehe (:MarcoZ)]

Can we force the accessible to be in the cache when we want to fire a focus event for it?

Comment 14

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

Yeah, we need to figure out why GetAccessibleByRule fails to do this.

Comment 15

[alexander :surkov (:asurkov)]

(In reply to comment #13)
> Can we force the accessible to be in the cache when we want to fire a focus
> event for it?

Perhaps we can iif we're sure layout is ready, i.e. lazy frame construction is forced when something (that is pended for frame construction) is going to get a focus. In this case bug 570275 is going to be a right fix for this. On the another hand we could do delayed focus event (and other DOM events) processing, though it might be tricky.

Comment 16

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

I was also thinking bug 570275 should fix this. As for delayed events, I still think all our events should be delayed/async but that might be a bit of a scary change for FF4 at this point.

Comment 17

[alexander :surkov (:asurkov)]

(In reply to comment #16)
> I was also thinking bug 570275 should fix this. As for delayed events, I still
> think all our events should be delayed/async but that might be a bit of a scary
> change for FF4 at this point.

just remark, the point is we would need a delayed processing of DOM events (the second approach) not accessible events firing but that's scary change as well.

Comment 18

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

I'll do some tests (tomorrow at latest). As you know, we're running of the refresh driver observing Flush_Display which should be good enough, unless painting can affect frames?!

Comment 19

[alexander :surkov (:asurkov)]

fixed by bug 570275