[webkit-reviews] review canceled: [Bug 79500] [GTK] Add support for window.showModalDialog in WebKit2GTK+ : [Attachment 129276] Patch proposal + Unit tests

[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 79500: [GTK] Add support for window.showModalDialog in WebKit2GTK+
https://bugs.webkit.org/show_bug.cgi?id=79500

Attachment 129276: Patch proposal + Unit tests
https://bugs.webkit.org/attachment.cgi?id=129276&action=review

------- Additional Comments from Mario Sanchez Prada <msanchez at igalia.com>
Attaching a new patch addressing the issues pointed out by Carlos, and rebased
against latest changes in trunk.

Sorry I'm not providing a ChangeLog entry properly filled yet because I have to
literally run out of the office now, will do in the next patch.

(In reply to comment #11)
> (From update of attachment 129276 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=129276&action=review
> 
> I prefer to use a WebKitSetting for that. The view can connect to ::notify of
the settings to call m_page->setCanRunModal()

Fixed.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:494
> > +	  * Emitted when JavaScript code calls
> > +	  * <function>window.showModalDialog</function>, right before
> > +	  * creating and running a new mainloop to block user interaction
> > +	  * in the parent webview until the current webview is finalized.
> 
> You should mention that this is called after ready signal in the newly
created view and that the view will run in modal mode until it's closed. You
should also mention that this signal is to allow the user to prepare the view
to run in modal mode, typically setting the modal hint and transient for.

Fixed

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1409
> > + * Enable the current web view to create and run modal dialogs from
> > + * JavaScript through the <function>window.showModalDialog</function>.
> 
> You should clarify here that if this is set to FALSE, when
window.showModalDialog(), no dialog will be created at all, so ::create signal
won't be emitted. In other words, this setting prevents create signal to be
emitted when window.showModalDialog() is used.

Fixed
 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:129
> > +	 gboolean   (* run_as_modal)	     (WebKitWebView	       
*web_view);
> 
> This is not boolean enymore

Fixed