Bug 226320 – [GTK] Try harder to find initial WebKitWebView size

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alice Mikhaylenko]

See the patch.

[Alice Mikhaylenko]

Created attachment 429860 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Alice Mikhaylenko]

Since we eliminate a 0,0 -> proper size change at the beginning, the patch further regresses https://bugs.webkit.org/show_bug.cgi?id=219202 as the view will not redraw on its own, so it depends on the patch from that bug.

[Adrian Perez]

Patch looks sensible to me, but I would like another pair of
eyes to take a look as well :)

[Michael Catanzaro]

Comment on attachment 429860 [details]
Patch

Looks like there is some fallout:

Unexpected failures (2)
    /WebKit2Gtk/TestWebKitWebView
        /webkit/WebKitWebView/snapshot
    /WebKit2Gtk/TestUIClient
        /webkit/WebKitWebView/window-properties

[Michael Catanzaro]

Comment on attachment 429860 [details]
Patch

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

This looks good to me.

I would add a comment though, to explain why we are doing this, why it's not going to return a perfect result but is better than (0, 0), and probably also link to this bug report so we don't remove it by mistake in the future.

BTW it's already mentioned in the patch changelog, but I'll link to it here as well: this is to fix https://gitlab.gnome.org/GNOME/epiphany/-/issues/1532.

> Source/WebKit/ChangeLog:10
> +        size, which will be (0, 0) because the drawing area ist still null

is

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:2376
> +    // everything is fine and we'll just use that

Nit: that. with a period.

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:2383
> +    // through the hierarchy and try to find a parent with non-0 size

Nit: size. with a period.

[Carlos Garcia Campos]

Comment on attachment 429860 [details]
Patch

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

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:2373
> +{
> +#if USE(GTK4)
> +    int width = gtk_widget_get_width(GTK_WIDGET(webViewBase));
> +    int height = gtk_widget_get_height(GTK_WIDGET(webViewBase));
> +#else
> +    int width = gtk_widget_get_allocated_width(GTK_WIDGET(webViewBase));
> +    int height = gtk_widget_get_allocated_height(GTK_WIDGET(webViewBase));
> +#endif

I think we could add gtk_widget_get_width/height to GtkVersioning for gtk3 that returns allocated width/height. Assuming we always want the allocated size in gtk3 when width/height is queried in gtk4

[Carlos Garcia Campos]

Comment on attachment 429860 [details]
Patch

Tests are not passing.

[Alice Mikhaylenko]

(In reply to Carlos Garcia Campos from comment #7)
> I think we could add gtk_widget_get_width/height to GtkVersioning for gtk3
> that returns allocated width/height. Assuming we always want the allocated
> size in gtk3 when width/height is queried in gtk4

I mean, really we want `get_width()` in both cases but it doesn't exist in GTK3, so `get_allocated_width()` is the closest we have.

[Alice Mikhaylenko]

(In reply to Carlos Garcia Campos from comment #8)
> Comment on attachment 429860 [details]
> Patch
> 
> Tests are not passing.

Great, they pass locally. I wonder what the difference is.

[Alice Mikhaylenko]

Nevermind, they pass for GTK4 and fail for GTK3. That makes more sense.

[Alice Mikhaylenko]

Created attachment 430194 [details]
Patch

Ok, one thing I hadn't known is that in GTK3 the default size is 1x1 and not 0x0. Another thing is I forgot about inspector which can take up a part of the web view size. Fixed both.

[Alice Mikhaylenko]

> I think we could add gtk_widget_get_width/height to GtkVersioning for gtk3 that returns allocated width/height. Assuming we always want the allocated size in gtk3 when width/height is queried in gtk4

I did this at first, but then reverted it - the latest version of the patch uses get_width()/height() only once and since the default size differs between GTK 3 and 4 I have an #if there anyway.

[Michael Catanzaro]

Comment on attachment 430194 [details]
Patch

Thanks!

[EWS]

Committed r278301 (238338@main): <https://commits.webkit.org/238338@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430194 [details].