[Webkit-unassigned] [Bug 137759] Handle Open Panel Functions Are Unimplemented

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=137759

Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #267951|commit-queue?               |commit-queue-
              Flags|                            |

--- Comment #28 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 267951
  --> https://bugs.webkit.org/attachment.cgi?id=267951
Patch

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

Added a few comments. I only did a very shallow review pass though.

> Source/WebKit2/UIProcess/API/Cocoa/WKUIDelegate.h:40
> +/*! A class conforming to WKOpenPanelResultListener provides methods methods

"methods methods"

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:253
> +    if (!m_uiDelegate.m_webView) {
> +        listener->invalidate();
> +        return false;
> +    }
> +
> +    NSWindow *window = [m_uiDelegate.m_webView window];

An explicit null check for m_uiDelegate.m_webView is not needed, please remove it. In Objective-C, sending a message to a nil target is supported, and results in a nil pointer in this case.

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:262
> +    // If we have no delegate, invalide the listener and return false.

How is this possible, given that m_uiDelegate.m_delegateMethods.webViewRunOpenPanelWithResultListenerAllowsMultipleFiles is true?

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:268
> +    [((id <WKUIDelegate>) delegate) webView:m_uiDelegate.m_webView runOpenPanelWithResultListener:(id <WKOpenPanelResultListener>)adoptNS([[WKConcreteOpenPanelResultListener alloc] initWithListenerProxy:listener]) allowsMultipleFiles:parameters->allowMultipleFiles()];

It shouldn't be necessary to typecast delegate here, m_delegate is "id <WKUIDelegate>" already.

> Tools/MiniBrowser/mac/WK2BrowserWindowController.m:452
> +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1090

Please remove this check, trunk WebKit doesn't support 10.9.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151230/90e2826e/attachment.html>