Bug 163292

Summary: Implement displaying context menu asynchronously
Product: WebKit Reporter: Damian Kaleta <dkaleta>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: beidson, commit-queue, mcatanzaro, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 10   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 213793    
Attachments:
Description Flags
Proposed patch.
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews112 for mac-yosemite
none
Patch none

Damian Kaleta
Reported 2016-10-11 12:32:59 PDT
Currently WebKit doesn't allow any client to display the context menu asynchronously. We should add ability for the client to provide the list of context menu items to WebKit asynchronously.
Attachments
Proposed patch. (31.47 KB, patch)
2016-10-13 16:38 PDT, Damian Kaleta
no flags
Patch (30.95 KB, patch)
2016-10-14 11:02 PDT, Damian Kaleta
no flags
Patch (32.02 KB, patch)
2016-10-17 12:10 PDT, Damian Kaleta
no flags
Patch (32.49 KB, patch)
2016-10-18 16:40 PDT, Damian Kaleta
no flags
Archive of layout-test-results from ews112 for mac-yosemite (1.57 MB, application/zip)
2016-10-18 18:02 PDT, Build Bot
no flags
Patch (35.19 KB, patch)
2016-10-19 10:24 PDT, Damian Kaleta
no flags
Damian Kaleta
Comment 1 2016-10-13 16:38:01 PDT
Created attachment 291536 [details] Proposed patch.
Damian Kaleta
Comment 2 2016-10-14 11:02:15 PDT
WebKit Commit Bot
Comment 3 2016-10-14 11:04:44 PDT
Attachment 291651 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/WebContextMenuListenerProxy.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5] ERROR: Source/WebKit2/UIProcess/API/C/WKContextMenuListener.h:26: Use #pragma once instead of #ifndef for header guard. [build/header_guard] [5] Total errors found: 2 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Brady Eidson
Comment 4 2016-10-14 16:33:02 PDT
Comment on attachment 291651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291651&action=review > Source/WebKit2/UIProcess/API/C/WKPage.cpp:857 > + if (m_client.base.version <= 3 || !m_client.getContextMenuFromProposedMenuAsync) How about: m_client.base.version < 4 It reads better, I think, referencing the version where the new bits were added. > Source/WebKit2/UIProcess/WebContextMenuListenerProxy.cpp:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. Update copyright for new files > Source/WebKit2/UIProcess/WebContextMenuListenerProxy.h:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. Update the copyright date for new files > Source/WebKit2/UIProcess/WebContextMenuListenerProxy.h:27 > +#ifndef WebContextMenuListenerProxy_h > +#define WebContextMenuListenerProxy_h #pragma once > Source/WebKit2/UIProcess/WebContextMenuListenerProxy.h:47 > + void useProposedContextMenuItems(WKArrayRef items); The initial call out to the WK client is "get context menu from proposed menu". So I wouldn't call the returned menu the "proposed menu" as it has assumedly undergone changes. I'd suggest "useContentMenuItems" > Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:470 > + if (m_contextMenuListener) { > + m_contextMenuListener->invalidate(); > + m_contextMenuListener = nullptr; > + } Very important to invalidate this in the destructor as well.
Damian Kaleta
Comment 5 2016-10-17 12:10:43 PDT
Damian Kaleta
Comment 6 2016-10-18 16:40:03 PDT
Build Bot
Comment 7 2016-10-18 18:02:13 PDT
Comment on attachment 292002 [details] Patch Attachment 292002 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2319358 New failing tests: imported/w3c/web-platform-tests/html/semantics/embedded-content/the-canvas-element/security.dataURI.html
Build Bot
Comment 8 2016-10-18 18:02:15 PDT
Created attachment 292015 [details] Archive of layout-test-results from ews112 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Brady Eidson
Comment 9 2016-10-19 08:43:04 PDT
For the iOS build breakage, you'll have to wrap new stuff in #if ENABLE(CONTEXT_MENUS) because they are disabled.
Brady Eidson
Comment 10 2016-10-19 08:48:11 PDT
For the linux build breakage, there's an unimplemented virtual function in the base class: virtual void showContextMenuWithItems(const Vector<WebContextMenuItemData>& items) = 0; You'll need to stub it out in WebContextMenuProxyGtk.h and WebContextMenuProxyEfl.h
Damian Kaleta
Comment 11 2016-10-19 10:24:09 PDT
WebKit Commit Bot
Comment 12 2016-10-19 12:48:32 PDT
Comment on attachment 292080 [details] Patch Clearing flags on attachment: 292080 Committed r207558: <http://trac.webkit.org/changeset/207558>
Michael Catanzaro
Comment 13 2018-01-15 12:44:44 PST
This seems to be fixed, right?
Radar WebKit Bug Importer
Comment 14 2018-01-15 12:45:31 PST
Note You need to log in before you can comment on or make changes to this bug.