393660 - When non-accessible node is removed/shown, fire events for accessible first-line descendants

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

For this case, when EVENT_SHOW is fired on the DOM node 

Accessible ancestor node:
    \
    DOM Node (not accessible)
    |            |                |
Accessible      Accessible      Accesssible

The 3 accessible descendants (which are accessible siblings but may not be DOM siblings) need the correct SHOW/HIDE events.

Comment 1

[alexander :surkov (:asurkov)]

I thought we fire events for ancestor node if the target node is not accessible.

Comment 2

[Aaron Leventhal]

Surkov, that's true for text change events.

But if the dom node in the description of this bug goes away, that means all 3 accessibles have become hidden. Wouldn't you expect hide events for those 3 accessibles? Or where would you expect the hide event to be fired? If it is fired on the ancestor that makes no sense, because the ancestor is still there.

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 4

[Aaron Leventhal]

This is not right:

  PRBool isHidden = aEventType == nsIAccessibleEvent::EVENT_ASYNCH_HIDE ||
                    aEventType == nsIAccessibleEvent::EVENT_ASYNCH_HIDE;

Misspelling (in both comments, variable names and method name)
s/descedants/descendants) 

Comment 5

[Aaron Leventhal]

Comment on attachment 282005 [details] [diff] [review]
patch

Waiting for a new patch, but also I want to wait until we deal with our crashes in the invalidation handling code, before we go further and work on this.

Comment 6

[Aaron Leventhal]

This fixes the invalid condition causing bug 394198 to happen.

Comment 7

[Aaron Leventhal]

Attachment: Updated to trunk, fixed wrong isHidden event type, and corrected spelling

Comment 8

[Aaron Leventhal]

Attachment: 1) Remove aDescendant -- it can fire for the main node, if that's accessible. This is consistent with the logic, 2) Avoid O(n^2) operations when visibility becomes hidden for a subtree

Surkov, I hope you don't mind, but I needed to work on this myself to understand that it is fundamentally correct. I've attached a changed version of your original patch.

Comment 9

[alexander :surkov (:asurkov)]

Comment on attachment 283070 [details] [diff] [review]
1) Remove aDescendant -- it can fire for the main node, if that's accessible. This is consistent with the logic, 2) Avoid O(n^2) operations when visibility becomes hidden for a subtree

looks fine