Bug 125783 – AX: WebKit not sending AXMenuClosed notification

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

AXMenuClosedNotification
https://dvcs.w3.org/hg/pfwg/raw-file/default/ARIA-UAIG/1.0/tests/test-files/test96.html

[chris fleizach]

<rdar://problem/15668928>

[chris fleizach]

Created attachment 219510 [details]
patch

[Mario Sanchez Prada]

Comment on attachment 219510 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=219510&action=review

> Source/WebCore/accessibility/AccessibilityObject.cpp:95
> +    // Menu close events need to notify the platform. No element is used in the notification because it's a destruction event.
> +    if (detachmentType == ElementDestroyed && roleValue() == MenuRole && cache)
> +        cache->postNotification(nullptr, &cache->document(), AXObjectCache::AXMenuClosed);

Would it be possible to use AccessibilityObject::document() here, so you would not need to add AXObjectCache::document()?

Also, would it be possible to use AccessibilityObject::axObjectCache(), instead of having to add that new first parameter to AccessibilityObject::detach()?

I guess the answer is that it's probably too late here to get the document (and hence the AXObjectCache out of it) from the AccessibilityObject that is being detached, but I have to ask anyway :)

If that's the case, maybe it would be cleaner to swap the arguments in detach() and add a default value for the cache, so we have something like void detach(AccessibilityDetachmentType, AXObjectCache* axObjectCache = nullptr) that allows you to omit that "mysterious" nullptr parameter when not using ElementDestroyed as the "detachment type"

[chris fleizach]

(In reply to comment #3)
> (From update of attachment 219510 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=219510&action=review
> 
> > Source/WebCore/accessibility/AccessibilityObject.cpp:95
> > +    // Menu close events need to notify the platform. No element is used in the notification because it's a destruction event.
> > +    if (detachmentType == ElementDestroyed && roleValue() == MenuRole && cache)
> > +        cache->postNotification(nullptr, &cache->document(), AXObjectCache::AXMenuClosed);
> 
> Would it be possible to use AccessibilityObject::document() here, so you would not need to add AXObjectCache::document()?
> 
> Also, would it be possible to use AccessibilityObject::axObjectCache(), instead of having to add that new first parameter to AccessibilityObject::detach()?
> 
> I guess the answer is that it's probably too late here to get the document (and hence the AXObjectCache out of it) from the AccessibilityObject that is being detached, but I have to ask anyway :)

Correct

> 
> If that's the case, maybe it would be cleaner to swap the arguments in detach() and add a default value for the cache, so we have something like void detach(AccessibilityDetachmentType, AXObjectCache* axObjectCache = nullptr) that allows you to omit that "mysterious" nullptr parameter when not using ElementDestroyed as the "detachment type"

Yep. No problem

[Mario Sanchez Prada]

Comment on attachment 219510 [details]
patch

(In reply to comment #4)
> [...]
> > If that's the case, maybe it would be cleaner to swap the arguments in detach() and add a default value for the cache, so we have something like void detach(AccessibilityDetachmentType, AXObjectCache* axObjectCache = nullptr) that allows you to omit that "mysterious" nullptr parameter when not using ElementDestroyed as the "detachment type"
> 
> Yep. No problem

Thanks for the confirmation. Setting r+ now

[chris fleizach]

https://trac.webkit.org/changeset/160778