Closed
Bug 689105
Opened 13 years ago
Closed 13 years ago
Accessibility in main window broken for VoiceOver, VO doesn't see anything but the title bar and its buttons
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: MarcoZ, Assigned: hub)
References
Details
(Keywords: regression, Whiteboard: [bk1])
Attachments
(2 files, 3 obsolete files)
(deleted),
patch
|
surkov
:
review+
davidb
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
STR:
1. Build current trunk with accessibility enabled.
2. Run the build.
3. Run VoiceOver.
4. If you get a "Nightly is not your current browser" dialog, notice that VoiceOver does see all the text in the dialog, plus the checkbox and the two buttons. You can press the No button with CTRL+Option+Space, for example.
5. Once you get to the main window, and Nightly displays its start page, VoiceOver does not see anything but the title bar and the close, minimize, and zoom buttons. Normally, it should also see toolbars, the main web area, and the status bar. But none of that is visible. You can find this out by navigating through the top level hierarchy with CTRL+Option+Arrow right and left.
This is a regression from some time in the Firefox 4 timeframe I believe, where I saw this working better. But finding a regression range is not really an option since no regular nightly build was ever built with accessibility enabled.
Comment 1•13 years ago
|
||
Steven, if you get a chance please look into. You might be a best person to detect where we regressed. Thank you for the help.
Comment 2•13 years ago
|
||
I'm currently swamped, so it'll be a while before I can get to this.
But until we get someone specifically working on Mac accessibility, it's probably true that I'm the best one to figure this out.
Updated•13 years ago
|
Assignee: nobody → hfiguiere
Assignee | ||
Updated•13 years ago
|
Whiteboard: [bk1]
Assignee | ||
Comment 3•13 years ago
|
||
To be honest, I just checked with a custom build of 3.6.24 and it is work than what is in Mozilla central. With Nightly build from mozilla central I can at least partially navigate through the webpage under some circumstances.
As for the dialog it isn't read either way, but we can use the VoiceOver navigation keys to dismiss it (buttons are read).
Definitely broken, not sure if it actually regressed.
Comment 4•13 years ago
|
||
Do a mozilla-central build from code just after my patch for bug 560939 landed.
Assignee | ||
Comment 5•13 years ago
|
||
Using the Accessibility Validator, we actually have a tree that "randomly" miss complete subtrees, consistently with the accessibility issue we observe.
Needs more digging as to why this happens.
There are also some glitches in the implementation that cause some errors with the Validator, the AXMain and AXMinimized property missing from the dialog, and the some extra unexpected properties. I don't think these are fatal but I plan on fixing them too.
Comment 6•13 years ago
|
||
(In reply to Hub Figuiere [:hub] from comment #5)
> There are also some glitches in the implementation that cause some errors
> with the Validator, the AXMain and AXMinimized property missing from the
> dialog, and the some extra unexpected properties. I don't think these are
> fatal but I plan on fixing them too.
please file separate bugs for that.
Assignee | ||
Comment 7•13 years ago
|
||
The proposed patch. It needs more testing, but I do believe this is the actual problem. It seems to solve all the issues I had with the hierarchy.
I need to test on a non debug build.
Comment 8•13 years ago
|
||
Comment on attachment 576392 [details] [diff] [review]
proposed patch
>diff --git a/accessible/src/mac/nsAccessibleWrap.mm b/accessible/src/mac/nsAccessibleWrap.mm
>--- a/accessible/src/mac/nsAccessibleWrap.mm
>+++ b/accessible/src/mac/nsAccessibleWrap.mm
>@@ -256,16 +256,19 @@ nsAccessibleWrap::IsIgnored()
>
> void
> nsAccessibleWrap::GetUnignoredChildren(nsTArray<nsRefPtr<nsAccessibleWrap> > &aChildrenArray)
> {
> // we're flat; there are no children.
> if (nsAccUtils::MustPrune(this))
> return;
>
>+ // make sure the children are here.
>+ EnsureChildren();
You never should call it explicitly. Children are cached automatically when accessibility starts but that happens async. That could be a problem. Maybe we should deliver then events that tree was changed instead? Can you describe in details what happens here?
Comment 9•13 years ago
|
||
Btw, it'll be great to get regression bug. I think you could bisect extension (http://mercurial.selenic.com/wiki/BisectExtension) for that.
Assignee | ||
Comment 10•13 years ago
|
||
What happens is that in some situation we don't have the children created. As you said it is done asynchronously and that's exactly what happens. Adding the "EnsureChildren()" make sure they are here when we query the accessible tree.
I guess we will have to deliver the events that tree was changed instead.
Assignee | ||
Comment 11•13 years ago
|
||
To be honest, I'm not sure how it was a regression per see. Comment 3.
Comment 12•13 years ago
|
||
(In reply to Hub Figuiere [:hub] from comment #10)
> I guess we will have to deliver the events that tree was changed instead.
If this is a cause then nsaccessibility prootocol provides events AXUIElementCreated and AXUIElementDestroyed. But that shouldn't be issue since we never delivered them. Did you checked if mac accessible children cache doesn't get updated by any chance?
Comment 13•13 years ago
|
||
(In reply to alexander surkov from comment #12)
> Did you checked if mac accessible children
> cache doesn't get updated by any chance?
For example, nsAccessibleWrap::InvalidateChildren doesn't trigger on nsOuterDocAccessible, what could lead for example tab document accessible is not presented in mac cached children.
Comment 14•13 years ago
|
||
(In reply to alexander surkov from comment #13)
> (In reply to alexander surkov from comment #12)
> > Did you checked if mac accessible children
> > cache doesn't get updated by any chance?
>
> For example, nsAccessibleWrap::InvalidateChildren doesn't trigger on
> nsOuterDocAccessible, what could lead for example tab document accessible is
> not presented in mac cached children.
yes, the problem mac cached children (mChildren) on root accessible weren't invalidated.
Comment 15•13 years ago
|
||
(In reply to alexander surkov from comment #14)
> yes, the problem mac cached children (mChildren) on root accessible weren't
> invalidated.
and those debug observations are right because initial tree creation doesn't trigger invalidateChildren. That'll be a fix.
Comment 16•13 years ago
|
||
btw, invalidateChildren approach doesn't work in either case because children invalidation on ignored accessible that has unignored children doesn't update mac children cache of the parent.
Comment 17•13 years ago
|
||
regression from bug 646368 which is sort of expected (see bug 705404 comment #5)
Blocks: 646368
Assignee | ||
Comment 18•13 years ago
|
||
This the new patch. Almost the same as the old one except that this time EnsureChildren() is called in -[mozAccessible children]
nsAccessibleWrap::GetUnignoredChildren() is only called from that location anyway.
-[mozAccessible children] is only called from the MacOS Universal Access API and caches the result until invalidated by a tree update.
Attachment #576392 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Attachment #578444 -
Flags: review?(surkov.alexander)
Comment 19•13 years ago
|
||
Comment on attachment 578444 [details] [diff] [review]
proposed patch
Layout is responsible for time of accessible tree creation. The patch introduces extra code path for that and put the creation time out of our control. It's harder to debug any broken tree issues and it can lead to broken accessible tree because we operate on unfinished layout. So I wouldn't try this approach as long as we have alternative.
Attachment #578444 -
Flags: review?(surkov.alexander) → review-
Assignee | ||
Comment 20•13 years ago
|
||
I had to expose "IsInitialized" so that I don't cache an empty children list.
Attachment #578444 -
Attachment is obsolete: true
Assignee | ||
Updated•13 years ago
|
Attachment #578684 -
Flags: review?(surkov.alexander)
Assignee | ||
Comment 21•13 years ago
|
||
Comment on attachment 578684 [details] [diff] [review]
proposed patch
cancelling review. There is a typo....
Attachment #578684 -
Flags: review?(surkov.alexander)
Assignee | ||
Comment 22•13 years ago
|
||
Attachment #578684 -
Attachment is obsolete: true
Attachment #578750 -
Flags: review?
Assignee | ||
Updated•13 years ago
|
Attachment #578750 -
Flags: review? → review?(surkov.alexander)
Assignee | ||
Updated•13 years ago
|
Attachment #578750 -
Flags: review?(bolterbugz)
Assignee | ||
Comment 23•13 years ago
|
||
This fix the VoiceOver at startup. But reloading the page, or going to a new one make it empty again. This is also the case before the regression.
Comment 24•13 years ago
|
||
Comment on attachment 578750 [details] [diff] [review]
proposed patch v4.1
> - (NSArray*)children
> {
> NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NIL;
>
> if (mChildren)
> return mChildren;
>-
>+
>+ if(!mGeckoAccessible->IsInitialized())
>+ return nil;
use AreChildrenCached
if (mChildren || !mGeckoAccessible-AreChildrenCached())
return mChildren;
Attachment #578750 -
Flags: review?(surkov.alexander) → review+
Updated•13 years ago
|
Attachment #578750 -
Flags: review?(bolterbugz) → review+
Assignee | ||
Comment 25•13 years ago
|
||
Can't commit with Level 1.
Has been run on a try server.
Attachment #579167 -
Flags: checkin?
Comment 26•13 years ago
|
||
Updated•13 years ago
|
Attachment #579167 -
Flags: checkin?
Comment 27•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
You need to log in
before you can comment on or make changes to this bug.
Description
•