Closed Bug 574588 Opened 14 years ago Closed 14 years ago

nsAccessible::GetRole() should avoid a11y tree traversal

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b7

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

CacheChildren implementation tends to get accessible role to decide whether accessible can be a child or not. nsAccessible::GetRole() implementation may trigger GetParent() while we didn't set GetParent() yet. That prevents us to make GetParent() return cached parent only and results in assertions (nsAccessible::GetParent()).

Bug 545465 is related with this one.
Blocks: 576931
Version: unspecified → Trunk
Summary: no nsAccessible method on child should be called during CacheChildren → nsAccessible::GetRole() should avoid a11y tree traversal
Attached patch patchSplinter Review
use cached parent, if it wasn't initialize then return some fallback role
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #470706 - Flags: review?(bolterbugz)
Attachment #470706 - Flags: approval2.0?
Comment on attachment 470706 [details] [diff] [review]
patch

>--- a/accessible/src/xul/nsXULMenuAccessible.cpp
>+++ b/accessible/src/xul/nsXULMenuAccessible.cpp

> 
>-  if (nsAccUtils::Role(GetParent()) == nsIAccessibleRole::ROLE_COMBOBOX_LIST) {
>+  if (nsAccUtils::Role(mParent) == nsIAccessibleRole::ROLE_COMBOBOX_LIST) {

> nsXULMenupopupAccessible::GetRoleInternal(PRUint32 *aRole)
> {

>-  nsAccessible *parent = GetParent();
>-  if (parent) {
>-    PRUint32 role = nsAccUtils::Role(parent);
>+  // If accessible is not bound to the tree (this happens while children are
>+  // cached) return general role.
>+  if (mParent) {
>+    PRUint32 role = nsAccUtils::Role(mParent);

How was GetParent asserting during child caching? Can you describe the problem and fix since I haven't debugged this? :)
(In reply to comment #2)

> How was GetParent asserting during child caching? Can you describe the problem
> and fix since I haven't debugged this? :)

while we cache children we check the role of child before binding it to the tree, role on the child tries to get parent and since there's no parent it tries to invalidate parent children and recache them, recaching fails because of recursion, so we get a broken tree (calculated parent doesn't have this child accessible in children) and assert about that.
Comment on attachment 470706 [details] [diff] [review]
patch

r+a=me
Attachment #470706 - Flags: review?(bolterbugz)
Attachment #470706 - Flags: review+
Attachment #470706 - Flags: approval2.0?
Attachment #470706 - Flags: approval2.0+
landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/cbad7ce2bf26
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Future
Target Milestone: Future → mozilla2.0b6
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: