[webkit-reviews] review granted: [Bug 210206] [GTK] MiniBrowser opens new windows too small causing failures on some WPT tests : [Attachment 396454] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Carlos Alberto Lopez
Perez <clopez at igalia.com>'s request for review:
Bug 210206: [GTK] MiniBrowser opens new windows too small causing failures on
some WPT tests
https://bugs.webkit.org/show_bug.cgi?id=210206

Attachment 396454: Patch

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




--- Comment #14 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 396454
  --> https://bugs.webkit.org/attachment.cgi?id=396454
Patch

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

> Source/WebCore/ChangeLog:11
> +	   The size of the new window is not specified, and we were failimg

failimg -> failing

> Source/WebCore/loader/FrameLoader.cpp:4129
> +#if PLATFORM(GTK) || PLATFORM(WPE)

I'm not sure this works in WPE. It's unfortunate we have to do this here, but
we don't have access to the previous window from the API layer.

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:370
> +#if PLATFORM(GTK)
> +	   if (m_shouldCreateWebViewsInNewWindowsAutomatically) {
> +	       g_assert_null(m_parentWindow);
> +	       m_parentWindow = gtk_window_new(GTK_WINDOW_TOPLEVEL);
> +	       gtk_window_set_default_size(GTK_WINDOW(m_parentWindow),
m_DefaultGeometryNewWindows.width, m_DefaultGeometryNewWindows.height);
> +	       gtk_container_add(GTK_CONTAINER(m_parentWindow),
GTK_WIDGET(newWebView));
> +	       gtk_widget_show(GTK_WIDGET(newWebView));
> +	       gtk_widget_show(m_parentWindow);
> +	   }
> +#endif

Maybe we can just change showInWindowAndWaitUntilMapped() to set the default
size, I don't think that will affect other tests. Ok, I see now that we need it
for this particular test. Then I would add a parameter ShouldSetDefaultSize or
something similar.

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:705
> +    // if no size specified for window.open(), then new windows open with
the default window size.

If

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:715
> +    // if no size specified for window.open(), and new windows are not set
to a specific default size with gtk_window_set_default_size()

If