Closed Bug 653607 Opened 9 years ago Closed 9 years ago

IAccessible::accChild fails on ARIA documents

Categories

(Core :: Disability Access APIs, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla6

People

(Reporter: Jamie, Assigned: surkov)

References

()

Details

(Keywords: access)

Attachments

(3 files, 1 obsolete file)

Bug 592913 changed IAccessible::accChild so that it fails if it is called on anything other than a document accessible. In general, this is fine. Unfortunately, this also means that it fails if called on an ARIA document (@role="document"). This causes problems for ARIA documents inside ARIA applications. NVDA renders the document separately in this case, so it expects accChild to work on the ARIA document accessible. (In fact, NVDA can't distinguish between ARIA documents and normal documents.)

It'd be great if IAccessible::accChild could be made to work on ARIA documents just as it does for normal documents; i.e. only return an object if it is a descendant of the document.

Str:
1. Open the URL specified in this bug.
2. Retrieve the ARIA document accessible (div with @role="document"; second child of the top document).
3. Retrieve the unique ID of its first child.
4. Call IAccessible::accChild on the ARIA document accessible (step 2), passing it the unique ID of its first child (step 3).
Expected: The first child of the ARIA document accessible should be returned.
Actual: E_INVALIDARG is returned.

Related NVDA issue ticket: http://www.nvda-project.org/ticket/1452
I'm not sure how feasible it is for Mozilla to do this. If it is an insane amount of work, it's probably easier for us to just crawl the hierarchy looking for the document. That isn't particularly elegant or performant (as noted in bug 592913), but I'm keenly aware that there are many larger issues that need addressing at present. :)
Marco, what is the likelihood of getting a fix for this on the Mozilla side soon? If it's low, we'll try to just work around it in NVDA.
Alex, can you take a look at this? Do you have any idea what's going on?
Jamie, should I care about performance or it could be unperformant solution?
I guess that depends how unperformant. :) We could call accChild on the document for every object that gets focus within the document's HWND. However, even if you're crawling up the parents, it's still probably more performant on the Mozilla side than it is for us to do this out-of-process, so I still think it'd be better.
(In reply to comment #5)
> I guess that depends how unperformant. :) We could call accChild on the
> document for every object that gets focus within the document's HWND.

Did you measure this? (Or get a feel for it?)
Scratch that. We don't really want you relying on HWNDs anyway.
(In reply to comment #6)
> > We could call accChild on the
> > document for every object that gets focus within the document's HWND.
Clarification on that statement: I mean that in the worst case, we could do this; we already do this. In other words, any solution needs to take into account that we'll call it at least on every focus change and possibly in more than one document.

> Did you measure this? (Or get a feel for it?)
We already call accChild a lot. However, if we implement our work around, we'll have to crawl the hierarchy more than we do now. It looks as if we are going to have to do this for Yahoo! anyway, so this bug would just make it faster when it is implemented.

(In reply to comment #7)
> Scratch that. We don't really want you relying on HWNDs anyway.
We just use the HWND as a "return early" optimisation. If a focus change happens in a completely different application, there's no point in us asking a Firefox document whether the focused object is a descendant.
Attached patch patch (obsolete) — Splinter Review
try server build: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.alexander@gmail.com-18e2acabbe5d
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #533580 - Flags: review?(bolterbugz)
Comment on attachment 533580 [details] [diff] [review]
patch

r=me. This looks right to me.

>+    // Document.
>+    if (IsDoc())
>+      return AsDoc()->GetAccessibleByUniqueIDInSubtree(uniqueID);
>+
>+    // ARIA document.
>+    if (ARIARole() == nsIAccessibleRole::ROLE_DOCUMENT) {
>+      nsDocAccessible* document = GetDocAccessible();
>+      nsAccessible* child = document->GetAccessibleByUniqueIDInSubtree(uniqueID);
>+      for (nsAccessible* parent = child->GetParent();
>+          parent && parent != document; parent = parent->GetParent()) {
>+        if (parent == this)
>+          return child;
>+      }
>+    }

Optional:
I spent 5 minutes here (before coffee) since I didn't know what the purpose of the loop was. Maybe add a comment that we need to check that the child is a child of this aria document? ( Then I'll spend only 1 min next time :) )
Attachment #533580 - Flags: review?(bolterbugz) → review+
1. Open this file with the try-server build and NVDA running.
2. Tab from the textbox to the document.
3. Close the tab with CTRL+W.

Result: Crash.
Comment on attachment 533580 [details] [diff] [review]
patch

>+    // ARIA document.
>+    if (ARIARole() == nsIAccessibleRole::ROLE_DOCUMENT) {
>+      nsDocAccessible* document = GetDocAccessible();
>+      nsAccessible* child = document->GetAccessibleByUniqueIDInSubtree(uniqueID);

I think we need to null check child here. r=me can stay if that's fixed.

>+      for (nsAccessible* parent = child->GetParent();
Attached patch patch to landSplinter Review
Attachment #533580 - Attachment is obsolete: true
landed - http://hg.mozilla.org/mozilla-central/rev/03dc9705a551
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Verified fixed in: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110523 Firefox/6.0a1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.