[webkit-reviews] review denied: [Bug 207372] [WPE][WebDriver] Implement user prompt support : [Attachment 393980] Updated patch after using unref instead of close.

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Lauro Moura
<lmoura at igalia.com>'s request for review:
Bug 207372: [WPE][WebDriver] Implement user prompt support
https://bugs.webkit.org/show_bug.cgi?id=207372

Attachment 393980: Updated patch after using unref instead of close.

https://bugs.webkit.org/attachment.cgi?id=393980&action=review




--- Comment #14 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 393980
  --> https://bugs.webkit.org/attachment.cgi?id=393980
Updated patch after using unref instead of close.

View in context: https://bugs.webkit.org/attachment.cgi?id=393980&action=review

> Source/WebKit/UIProcess/API/wpe/WebKitScriptDialogWPE.cpp:26
> +// As WPE has currently no public API to allow the browser to respond to
these commands,

This is not true, WebKitWebView::script-dialog is available in WPE and
WebKitScriptDialog is exposed too.

> Source/WebKit/UIProcess/API/wpe/WebKitScriptDialogWPE.cpp:36
> +    auto dialog_type = webkit_script_dialog_get_dialog_type(dialog);
> +    if (dialog_type == WEBKIT_SCRIPT_DIALOG_CONFIRM || dialog_type ==
WEBKIT_SCRIPT_DIALOG_BEFORE_UNLOAD_CONFIRM)
> +	   webkit_script_dialog_confirm_set_confirmed(dialog, TRUE);
> +    // W3C WebDriver tests expect an empty string instead of a null one when
the prompt is accepted.
> +    if (dialog_type == WEBKIT_SCRIPT_DIALOG_PROMPT && dialog->text.isNull())
> +	   webkit_script_dialog_prompt_set_text(dialog, "");
> +    webkit_script_dialog_unref(dialog);

This is assuming the app hasn't handled the WebKitWebView::script-dialog. In
GTK we use the WebKitScriptDialog::nativeDialog pointer to check if the dialog
is the default one or noty. We would need something similar for WPE, I think.