[webkit-reviews] review canceled: [Bug 84018] [GTK][WK2] Implement API for generic permission requests : [Attachment 137312] Patch Proposal

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <msanchez at igalia.com> has canceled Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 84018: [GTK][WK2] Implement API for generic permission requests
https://bugs.webkit.org/show_bug.cgi?id=84018

Attachment 137312: Patch Proposal
https://bugs.webkit.org/attachment.cgi?id=137312&action=review

------- Additional Comments from Mario Sanchez Prada <msanchez at igalia.com>
Thanks for the review. Attached new patch addressing all the issues.

See some comments below...

(In reply to comment #5)
> [...]
> Not all embedders are web browsers
>
Fixed.

> > Source/WebKit2/UIProcess/API/gtk/WebKitPermissionRequest.cpp:33
> > + * #WebKitWebVice::permission-request signal with a
> 
> WebKitWebVice? :-P

You have to admit it was a great name to have in the API, a bit useless though.


Fixed.
 
> > Source/WebKit2/UIProcess/API/gtk/WebKitPermissionRequest.cpp:40
> > + * If the signal handler does nothing, WebKit will act as if
> > + * webkit_permission_request_deny() was called as soon as signal
> > + * handling completes. To handle a permission request in an
> > + * asynchronous way, simply increment the reference count of the
> > + * #WebKitPermissionRequest object.
> 
> This depends on the implementation, or it should be enforced by the
interface.

Fixed (just removed)

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:656
> > +	  *
> 
> You should mention here that if this signal is not hanlded, the default
behaviour is to deny the request
>
Fixed.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:661
> > +	  *
> 
> A code snippet here would help to understand how to use this signal
>

Fixed.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:146
> > +	 gboolean   (* permission_request)    (WebKitWebView		 
*web_view,
> > +					       WebKitPermissionRequest	 
*permission_request);
> 
> There's an extra space before parameter names.

Fixed.