<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - Handle Open Panel Functions Are Unimplemented"
href="https://bugs.webkit.org/show_bug.cgi?id=137759#c30">Comment # 30</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - Handle Open Panel Functions Are Unimplemented"
href="https://bugs.webkit.org/show_bug.cgi?id=137759">bug 137759</a>
from <span class="vcard"><a class="email" href="mailto:brian.michel@gmail.com" title="Brian Michel <brian.michel@gmail.com>"> <span class="fn">Brian Michel</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=137759#c28">comment #28</a>)
<span class="quote">> Comment on <span class="bz_obsolete"><a href="attachment.cgi?id=267951&action=diff" name="attach_267951" title="Patch">attachment 267951</a> <a href="attachment.cgi?id=267951&action=edit" title="Patch">[details]</a></span>
> Patch
>
> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=267951&action=review">https://bugs.webkit.org/attachment.cgi?id=267951&action=review</a>
>
> 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.</span >
Thank you so much for your feedback Alexey, I've made the changes asked for, and tests on my local seem to be passing, but waiting for the bots to say so.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>