Closed Bug 646369 Opened 9 years ago Closed 9 years ago

UpdateTree should rely on accessible tree walker rather than DOM tree traversal

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(1 file)

nsDocAccessible::UpdateTree() navigates through DOM tree when search for accessible nodes. That's wrong since logic used to create an accessible (see nsAccTreeWalker) is much different than plain DOM tree traversal. As result the current approach doesn't work when XBL bound element is not accessible but its anon content is accessible.

Blocks bug 630486 and bug 646368.
Attached patch patchSplinter Review
Attachment #523028 - Flags: review?(bolterbugz)
Comment on attachment 523028 [details] [diff] [review]
patch

Cool. I need to look this over some more, but in the meantime:

>+bool
>+NotificationController::ContentInsertion::
>+  InitChildList(nsIContent* aStartChildNode, nsIContent* aEndChildNode)

>+    // Notification triggers for content insertion even if no content was
>+    // inserted actually, check if the given content has a frame to discard
>+    // this case early.

Nit: I prefer "even if not content was actually inserted,". Also, when does this happen (example)?

>+  <style>
>+    div.displayNone a { display:none; }
>+    div.visibilityHidden a { visibility:hidden; }
>+  </style>

How are these styles used?
(In reply to comment #2)

> >+    // Notification triggers for content insertion even if no content was
> >+    // inserted actually, check if the given content has a frame to discard
> >+    // this case early.
> 
> Nit: I prefer "even if not content was actually inserted,".

sure

> Also, when does
> this happen (example)?

iirc, anytime, when you do "display: none", it triggers pair of content removed and content inserted.
 
> >+  <style>
> >+    div.displayNone a { display:none; }
> >+    div.visibilityHidden a { visibility:hidden; }
> >+  </style>
> 
> How are these styles used?

nope, thanks for the catch.
Comment on attachment 523028 [details] [diff] [review]
patch

r=me.

>   nsAccTreeWalker(nsIWeakReference *aShell, nsIContent *aNode, 
>-                  PRBool aWalkAnonymousContent);
>+                  PRBool aWalkAnonymousContent, bool aWalkCache = false);

Can you file a bug as per IRC about separating walking usage vs tree creation?
Attachment #523028 - Flags: review?(bolterbugz) → review+
landed on cedar - http://hg.mozilla.org/projects/cedar/rev/883645192a06
Whiteboard: [fixed-in-cedar]
Flags: in-testsuite+
http://hg.mozilla.org/mozilla-central/rev/883645192a06
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-cedar]
Target Milestone: --- → mozilla2.2
You need to log in before you can comment on or make changes to this bug.