Bug 123649 - AX: AXShowMenu doesn't always work.
Summary: AX: AXShowMenu doesn't always work.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.9
: P2 Normal
Assignee: Samuel White
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-11-01 17:20 PDT by Samuel White
Modified: 2013-11-04 18:12 PST (History)
11 users (show)

See Also:


Attachments
Patch for discussion. (4.17 KB, patch)
2013-11-04 14:22 PST, Samuel White
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (439.44 KB, application/zip)
2013-11-04 15:23 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 (436.31 KB, application/zip)
2013-11-04 16:15 PST, Build Bot
no flags Details
Patch. (7.11 KB, patch)
2013-11-04 16:22 PST, Samuel White
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (445.02 KB, application/zip)
2013-11-04 17:15 PST, Build Bot
no flags Details
Patch for review (2.09 KB, patch)
2013-11-04 17:30 PST, Samuel White
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel White 2013-11-01 17:20:44 PDT
Sending AXShowMenu often fails.
Comment 1 Radar WebKit Bug Importer 2013-11-01 17:21:18 PDT
<rdar://problem/15375692>
Comment 2 Samuel White 2013-11-04 14:22:34 PST
Created attachment 215953 [details]
Patch for discussion.

Attached is a patch that fixes the described issue. We let the mouse event handler take a crack at the platform event before the context menu machinery. This is more inline with what actually happens during a right click event. Therefore, the targeted accessibility element will actually get focus as expected.

The problem I'd like feedback on is the test. testRunner.notifyDone is never called (inside finishJSTest) because the actual context menu shows and doesn't let things finish. I've looked at the non accessibility code path for testing context menus and noticed they don't actually show the context menu like we're trying to do in accessibility land. Instead, they return an array of what WOULD show. Should we be doing something similar to avoid this platform level lockup?

Any thoughts/help here would be great. Thanks!
Comment 3 Samuel White 2013-11-04 14:29:13 PST
To elaborate a bit more, this test currently works, but it simply isn't terminated correctly. Output:

stopping DumpRenderTree(pid 34298) timed out, killing it
The test ran as expected.

Again, notifyDone isn't completing as expected.
Comment 4 chris fleizach 2013-11-04 14:40:27 PST
Comment on attachment 215953 [details]
Patch for discussion.

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

> LayoutTests/platform/mac/accessibility/show-context-menu.html:29
> +        setTimeout(function() {

I think we want to call setTimeout before showMenu (maybe)
Comment 5 Samuel White 2013-11-04 15:06:40 PST
(In reply to comment #4)
> (From update of attachment 215953 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=215953&action=review
> 
> > LayoutTests/platform/mac/accessibility/show-context-menu.html:29
> > +        setTimeout(function() {
> 
> I think we want to call setTimeout before showMenu (maybe)

Sadly, doesn't help.
Comment 6 Build Bot 2013-11-04 15:23:58 PST
Comment on attachment 215953 [details]
Patch for discussion.

Attachment 215953 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/19568841

New failing tests:
platform/mac/accessibility/show-context-menu.html
Comment 7 Build Bot 2013-11-04 15:23:59 PST
Created attachment 215959 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 8 Samuel White 2013-11-04 15:41:02 PST
Confirmed the DRT function showMenu caused the exact same issue before this patch.
Comment 9 Build Bot 2013-11-04 16:15:28 PST
Comment on attachment 215953 [details]
Patch for discussion.

Attachment 215953 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/19618851

New failing tests:
platform/mac/accessibility/show-context-menu.html
Comment 10 Build Bot 2013-11-04 16:15:30 PST
Created attachment 215964 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 11 Samuel White 2013-11-04 16:22:26 PST
Created attachment 215966 [details]
Patch.

Please see the detailed comments I've put in the Changelogs.
Comment 12 Build Bot 2013-11-04 17:15:30 PST
Comment on attachment 215966 [details]
Patch.

Attachment 215966 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/19618871

New failing tests:
platform/mac/accessibility/show-context-menu.html
Comment 13 Build Bot 2013-11-04 17:15:31 PST
Created attachment 215976 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 14 Samuel White 2013-11-04 17:30:21 PST
Created attachment 215977 [details]
Patch for review

Removing useless testing and supplementing with better comments on the why and how.
Comment 15 WebKit Commit Bot 2013-11-04 18:12:37 PST
Comment on attachment 215977 [details]
Patch for review

Clearing flags on attachment: 215977

Committed r158629: <http://trac.webkit.org/changeset/158629>
Comment 16 WebKit Commit Bot 2013-11-04 18:12:40 PST
All reviewed patches have been landed.  Closing bug.