[Webkit-unassigned] [Bug 171100] Implement the Web Share API
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Aug 27 20:04:04 PDT 2018
https://bugs.webkit.org/show_bug.cgi?id=171100
--- Comment #53 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 348241
--> https://bugs.webkit.org/attachment.cgi?id=348241
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=348241&action=review
I think this is looking pretty good. Just some minor comments below:
> Source/WebCore/page/Navigator.cpp:135
> + return;
Nit - you don't need the return down here.
> Source/WebKit/UIProcess/API/C/WKPreferencesRefPrivate.h:500
> +// Defaults to true
It seems like this only defaults to true on iOS?
> Source/WebKit/UIProcess/PageClient.h:205
> + virtual bool showShareSheet(WebPageProxy*, const WebCore::ShareDataWithParsedURL&, WTF::CompletionHandler<void (bool)>&&) { return false; }
It doesn't seem like the first argument here (WebPageProxy*) is needed.
> Source/WebKit/UIProcess/WebPageProxy.cpp:4593
> + CompletionHandler<void(bool)> completionHandler = [this, protectedThis = makeRef(*this), callbackID] (bool access) {
I don't think you need to capture this as well as protectedThis; you can just capture protectedThis and then use protectedThis->m_process and protectedThis->m_pageID.
> Source/WebKit/UIProcess/WebPageProxy.h:1479
> + void runShareSheet(const WebCore::ShareDataWithParsedURL&, uint64_t callbackID);
ShareSheetCallbackID!
> Source/WebKit/WebProcess/WebPage/WebPage.h:1078
> + void runShareSheetResponse(bool wasCompleted, uint64_t contextId);
ShareSheetCallbackID!
> LayoutTests/resources/ui-helper.js:248
> + const resolveShareSheet = `(() => uiController.invokeShareSheetWithResolution(${resolved}))()`;
Nit - still missing one level of indentation here.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180828/a01c5937/attachment.html>
More information about the webkit-unassigned
mailing list