[webkit-reviews] review canceled: [Bug 79500] [GTK] Add support for window.showModalDialog in WebKit2GTK+ : [Attachment 129722] 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 129722: Patch proposal + Unit tests
https://bugs.webkit.org/attachment.cgi?id=129722&action=review

------- Additional Comments from Mario Sanchez Prada <msanchez at igalia.com>
(In reply to comment #13)
> (From update of attachment 129722 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=129722&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:850
> > +							      TRUE,

Agreed. Fixed.

> I think this should be FALSE by default. Otherwise an app not handling
::run-as-modal would end up with a main loop blocking a non modal dialog
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:176
> > +static void allowModalDialogsChanged(GObject* object, GParamSpec*,
WebKitWebView* webView)
> 
> You casted this in g_signal_connect, so you can use directly WebKitSettings
*settings, so that you don't need neither a cast nor a local variable.

I'm a moron. Fixed (the extra cast, not me being a moron. You can file a bug
for that if you want, though)

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:538
> > +	  * Emitted after #WebKitWebView::create and
> > +	  * #WebKitWebView::ready-to-show on the newly created
> > +	  * #WebKitWebView when JavaScript code calls
> 
> This is probably a bit confusing, it might look like create is called in the
newly created view, I would leave only Emitted after
#WebKitWebView::ready-to-show on the newly created #WebKitWebView.

It is definitely confusing. Fixed too.

Also, the new patch features a ChangeLog again. Reviewers won't have any excuse
now! :-)