IAccessible::accChild fails on ARIA documents

VERIFIED FIXED in mozilla6

Status

()

defect
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: Jamie, Assigned: surkov)

Tracking

({access})

Trunk
mozilla6
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

8 years ago
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
Reporter

Comment 1

8 years ago
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. :)
Reporter

Comment 2

8 years ago
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?
Assignee

Comment 4

8 years ago
Jamie, should I care about performance or it could be unperformant solution?
Reporter

Comment 5

8 years ago
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.
Reporter

Comment 8

8 years ago
(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.
Assignee

Comment 9

8 years ago
Posted 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();
Assignee

Comment 14

8 years ago
Posted patch patch to landSplinter Review
Attachment #533580 - Attachment is obsolete: true
Assignee

Comment 15

8 years ago
landed - http://hg.mozilla.org/mozilla-central/rev/03dc9705a551
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Reporter

Comment 16

8 years ago
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.