The default bug view has changed. See this FAQ.

IAccessible::accChild fails on ARIA documents

VERIFIED FIXED in mozilla6

Status

()

Core
Disability Access APIs
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Jamie, Assigned: surkov)

Tracking

({access})

Trunk
mozilla6
x86
Windows 7
access
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

6 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

6 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

6 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.

Comment 3

6 years ago
Alex, can you take a look at this? Do you have any idea what's going on?
(Assignee)

Comment 4

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

Comment 5

6 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.
(Reporter)

Updated

6 years ago
(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

6 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.

Updated

6 years ago
Blocks: 658181
(Assignee)

Comment 9

6 years ago
Created attachment 533580 [details] [diff] [review]
patch

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+
Created attachment 533621 [details]
Test file that causes a crash upon closing the tab.

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.
Created attachment 533625 [details]
stack
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

6 years ago
Created attachment 533889 [details] [diff] [review]
patch to land
Attachment #533580 - Attachment is obsolete: true
(Assignee)

Comment 15

6 years ago
landed - http://hg.mozilla.org/mozilla-central/rev/03dc9705a551
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
(Reporter)

Comment 16

6 years ago
Verified fixed in: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110523 Firefox/6.0a1
Status: RESOLVED → VERIFIED
Keywords: dev-doc-needed
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.