GNOME Bugzilla – Bug 471537
We need to find a way to identify truly "focusable" elements in FF3
Last modified: 2008-07-22 19:28:30 UTC
In the process of investigating an issue Mike pointed out, along with an issue Scott raised on bug 469894, I discovered that the overflow:auto; style is causing elements (sections, paragraphs, etc.) to have STATE_FOCUSABLE. This triggers an unwanted behavior in Orca thanks to this block of code in Gecko.py.setCaretPosition(): # We'd like the object to have focus if it can take focus. # Otherwise, we bubble up until we find a parent that can # take focus. This is to allow us to help force focus out # of something such as a text area and back into the # document content. # objectForFocus = obj while objectForFocus and obj: if objectForFocus.state.count(\ atspi.Accessibility.STATE_FOCUSABLE): break else: objectForFocus = objectForFocus.parent if objectForFocus: # (comments removed) focusGrabbed = objectForFocus.component.grabFocus() We don't want to be calling grabFocus() on sections -- or paragraphs or headings or anything else that isn't really focusable functionally. It causes Orca to get stuck on a block of text. It causes tables that had focus to lose focus. Etc. Etc. I thought this was a Mozilla bug and filed one to not have STATE_FOCUSABLE applied to objects simply due to overflow:auto. But, apparently, items with scroll bars need to have STATE_FOCUSABLE. Aaron pointed out an existing bug and patch that might help: https://bugzilla.mozilla.org/show_bug.cgi?id=257938 Should css scrollable areas without scrollbars be tabbable? I pinged the bug and a new patch has just been posted. (Yea!) If that gets approved, that will solve our problem for a lot of pages. However, it won't solve our problem when scroll bars are present (like Scott's table test case). So the question is, how can we check and distinguish really focusable from overflow:auto focusable? Aaron proposed one possibility: Remove STATE_ENABLED for these objects. Then we could check for both STATE_FOCUSABLE and STATE_ENABLED before deciding to grabFocus(). That would be great -- assuming that is not inconsistent with the defintion of STATE_ENABLED (which is vaguelly defined IMHO) and that it won't break anything else for us. :-) Comments?
Created attachment 94584 [details] [review] possible solution I had a phone conversation about this with Will and, Will being Will, he had a brilliant idea: 1. In setCaretPosition() Store the objectForFocus that we're about to grabFocus() on. 2. Do a check in onFocus(). If the source of the focus event is the object we did a grabFocus() on, quietly set the locusOfFocus to that object and then return. That was nearly perfect. Turns out we are also getting caret-moved events for the offending object. Scott already observed this behavior with the table grid bug (bug #469894). And to be honest, I should have known. We typically get the following series in this order: 1. focus: 2. object:state-changed:focused 3. object:text-caret-moved So, I added another check in onCaretMoved() and reset our state variable there instead of in onFocus() and, ya know, it seems to work: 1. It solves the table grid problem. 2. It solves us getting stuck on Mike's article sample: http://apcmag.com/node/7012 (We were getting stuck at the photo caption each time we arrowed from one paragraph to another) 3. I arrowed, tabbed, and structural-naved among various actual focusable things and I'm not seeing any side effects.... Will, Scott, what do you think? If we agree this is cool, a) Mike should hammer the heck out of it and b) we should probably tell Aaron. ;-) Ev
That's great if this patch works, but I always have a sour feeling when changes in one event handler directly affect the results of another event handler. The order in which the events are received becomes critically important then. The solution looks good until we find another web page that triggers a different order of events. I hope this doesn't happen and Firefox delivers us consistent events. I really feel that not having a standard order in which events are fired is a weakness in AT-SPI. Please point me to any documentation if there is such a standard. One question: Should we be using self.isSameObject(event.source, self._objectForFocusGrab) instead of event.source == self._objectForFocusGrab in the following line? if event.source and event.source == self._objectForFocusGrab:
To be on the safe side, yeah, probably. Good catch. And thanks! Assuming this is what we want to do, I'll make a new version that uses isSameObject().
> I really feel that not having a standard order in which events are fired is a > weakness in AT-SPI. Please point me to any documentation if there is such a > standard. Agreed. This has been a problem since day 1 of the service based approach to accessibility (i.e., RAP - the Remote Access Protocol). A tightly specified event order would allow us to do many more sophisticated things. For example, rather than listening for sentinel events, we could develop state machines driven by events. Another thing I would love to see is the ability to group a sequence of events caused by a single user action (e.g., a keypress). But, there are no plans to do these things, and I suspect it will be a long time before any such things happens. So, we're still in the land of listening for sentinel events: focus, selection, caret moved, etc. :-( > One question: > Should we be using self.isSameObject(event.source, self._objectForFocusGrab) > instead of event.source == self._objectForFocusGrab in the following line? > > if event.source and event.source == self._objectForFocusGrab: This is a great idea, especially because Firefox feels free to randomly play "Invasion of the Body Snatchers" and "To Tell the Truth" between delivery of events. Joanie - I think the patch is good. One way to clean it up might be to just do a global replace of "objectForFocus" with "self._objectForFocusGrab". The main concern I have with the patch, however, is if it changes any kind of user experience when arrowing to/from other things that can take focus: radio buttons, push buttons, combo boxes, lists (the kind you can select things in), links, etc. We need some thorough testing on those.
Created attachment 94637 [details] [review] revisions suggested by Scott and Will Mike please test (and test, and then test some more) :-)
*** Bug 469894 has been marked as a duplicate of this bug. ***
From all of my testing I think this patch is quite helpful. Hopefully I didn't miss anything.
Patch committed to both trunk and the gnome-2-20 branch. Moving to pending.
From Mike's comment in comment #7, I think we can mark this as verified. Please close as FIXED and thanks for your hard work!