Bug 56734 – AX: showContextMenu not working in WK2

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

showContextMenu no longer works in WK2 because the popup menu cannot be triggered directly

[chris fleizach]

rdar://9095026

[chris fleizach]

Created attachment 86299 [details]
patch

[Darin Adler]

Comment on attachment 86299 [details]
patch

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

> Source/WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:2338
> +    // In WK2 use the available crome client.

Typo, "chrome".

> Source/WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:2339
> +    // In WK1, the popup menu can be triggered directly.

Can’t we put this code into the chrome client for WebKit1 also? Is it important to keep the code here? It seems awkward to have different code for the two and I don’t see why.

> Source/WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:2341
> +        PlatformMouseEvent mouseEvent(clickPoint, clickPoint, RightButton, MouseEventPressed, 1, false, false, false, false, WTF::currentTime());

Should just be able to call currentTime rather than WTF::currentTime.

> Source/WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:2344
> +        bool handled = frameView->frame()->eventHandler()->sendContextMenuEvent(mouseEvent);
> +        if (handled)
> +            frameView->frame()->page()->chrome()->showContextMenu();

Any need to null check the frame or the page?

[chris fleizach]

Created attachment 86369 [details]
patch

[chris fleizach]

(In reply to comment #3)
 
> > Source/WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:2339
> > +    // In WK1, the popup menu can be triggered directly.
> 
> Can’t we put this code into the chrome client for WebKit1 also? Is it important to keep the code here? It seems awkward to have different code for the two and I don’t see why.
> 

I've moved the logic used to open the context menu from the AX code into showContextMenu() in the WebKit chrome client. Previously it had been empty.

[Darin Adler]

Comment on attachment 86369 [details]
patch

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

> Source/WebKit/mac/WebCoreSupport/WebChromeClient.mm:886
> +    WebCore::Page* page = [m_webView page];

Why does this need the WebCore:: prefix and nothing else in this function does?

> Source/WebKit/mac/WebCoreSupport/WebChromeClient.mm:891
> +    if (!controller)
> +        return;

I don’t think this can ever be 0, so this check is not needed.

[chris fleizach]

http://trac.webkit.org/changeset/81619