Share sheets from Share menus appear outside the browser window This is because we don't implement the appropriate delegates to tell them which window is the source. rdar://problem/20455592
Created attachment 250536 [details] Patch v1
Attachment 250536 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.mm:40: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebKit2/UIProcess/mac/WKSharingServicePickerDelegate.mm:34: "WebCore/NSSharingServiceSPI.h" already included at Source/WebKit2/UIProcess/mac/WKSharingServicePickerDelegate.mm:33 [build/include] [4] ERROR: Source/WebKit2/UIProcess/mac/WKSharingServicePickerDelegate.h:32: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 3 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 250544 [details] Patch v2
Comment on attachment 250544 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=250544&action=review Looks like this doesn’t compile yet. So obviously, please don’t land until it does! > Source/WebKit/mac/Misc/WebSharingServicePickerController.h:42 > + WebSharingServicePickerClient(WebView *); explicit please Also should make this constructor be protected. > Source/WebKit/mac/Misc/WebSharingServicePickerController.h:64 > +- (instancetype)initWithSharingServicePicker:(NSSharingServicePicker *)sharingServicePicker client:(WebSharingServicePickerClient*)pickerClient; Can client be a reference instead of a pointer? > Source/WebKit/mac/Misc/WebSharingServicePickerController.mm:44 > + : m_webView(webView) What prevents us from holding on to the WebView pointer after the WebView is deallocated? > Source/WebKit/mac/Misc/WebSharingServicePickerController.mm:106 > + _handleEditingReplacement = NO; I don’t think we need this. Objective-C initializes everything to NO. > Source/WebKit/mac/WebCoreSupport/WebContextMenuClient.h:41 > +class WebContextMenuClient : public WebCore::ContextMenuClient, public WebSharingServicePickerClient > { Should move the brace up one line. Can the derivation be private here instead of public? > Source/WebKit/mac/WebView/WebHTMLView.mm:3365 > + ContextMenuClient& contextMenuClient = page->contextMenuController().client(); I suggest using auto& here instead of uttering the type. Also probably should do the typecast here instead of down in the code below. > Source/WebKit/mac/WebView/WebHTMLView.mm:3367 > + Vector<ContextMenuItem> menuItemVector = contextMenuItemVector(coreMenu->platformDescription()); I suggest using auto here instead of uttering the type.
Created attachment 250650 [details] Patch v3 - Review feedback and fixing older builds
Going to work through the build failures with EWS, then prep to land.
Created attachment 250659 [details] Patch v4 - More feature guards.
Created attachment 250660 [details] Patch v5 - Still more feature guards
Created attachment 250661 [details] Patch v6 - Mo' better feature guards
Created attachment 250663 [details] Patch v7 - Potentially landable
Comment on attachment 250663 [details] Patch v7 - Potentially landable Clearing flags on attachment: 250663 Committed r182756: <http://trac.webkit.org/changeset/182756>
Landed - https://github.com/WebKit/WebKit/commit/95ef8bd83955487e2e8db1f9513bd0d8377d730d and didn't backed out. Marking this as "RESOLVED FIXED". Thanks!