[webkit-reviews] review granted: [Bug 71362] [GTK] Add support for javascript dialogs in WebKit2 GTK+ API : [Attachment 113315] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 71362: [GTK] Add support for javascript dialogs in WebKit2 GTK+ API
https://bugs.webkit.org/show_bug.cgi?id=71362

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

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=113315&action=review


> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:343
> +	* Emitted when JavaScript code calls <function>alert</function>. If the


alert -> window.alert

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:392
> +	* signal is not handled a message dialog with Ok and Cancel buttons and


Ok -> OK

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:109
> +	   htmlString = g_strdup_printf(htmlOnLoadFormat, "self.close();");
> +    else if (g_str_equal(path, "/javascript_alert")) {
> +	   GOwnPtr<char> alertDialogMessage(g_strdup_printf("alert('%s')",
kAlertDialogMessage));
> +	   htmlString = g_strdup_printf(htmlOnLoadFormat,
alertDialogMessage.get());
> +    } else if (g_str_equal(path, "/javascript_confirm")) {
> +	   GOwnPtr<char> alertDialogMessage(g_strdup_printf(jsConfirmFormat,
kConfirmDialogMessage));

Could you skip soup server here again and just load a string?

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebView.cpp:247
> +    g_main_loop_run(test->m_mainLoop);

It's a bit odd that the fixture takes care of ending the main loop, but the
test takes care of running it. Maybe you could add a method to the fixture
like: runMainLoopUntilAlertDialog.