Bug 235303 – [GTK] Implement native form validation bubbles for GTK4

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

[ChangSeok Oh]

This is a follow-up ticket for bug 234629. We enable native validation bubbles for gtk4.

[ChangSeok Oh]

Created attachment 449362 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 449362 [details]
Patch

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

> Source/WebCore/platform/gtk/ValidationBubbleGtk.cpp:68
> +#if USE(GTK4)
> +    gtk_label_set_wrap(GTK_LABEL(label), TRUE);
> +#else
>      gtk_label_set_line_wrap(GTK_LABEL(label), TRUE);
> +#endif

Include <WebCore/GtkVersioning.h> and just use gtk_label_set_line_wrap() without ifdefs.

[Adrian Perez]

Comment on attachment 449362 [details]
Patch

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

> Source/WebCore/platform/gtk/ValidationBubbleGtk.cpp:83
> +    gtk_popover_set_position(GTK_POPOVER(m_popover), GTK_POS_TOP);

Is it needed to move the “gtk_popover_set_position()“ call? Also, maybe it would be
possible to move the call after the “#endif” given that it is the same for both GTK3
and GTK4.

[ChangSeok Oh]

Created attachment 449426 [details]
Patch

[ChangSeok Oh]

Comment on attachment 449362 [details]
Patch

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

Thanks for your review.

>> Source/WebCore/platform/gtk/ValidationBubbleGtk.cpp:68
>> +#endif
> 
> Include <WebCore/GtkVersioning.h> and just use gtk_label_set_line_wrap() without ifdefs.

I was not aware of this abstraction. Good to know. Thanks.

>> Source/WebCore/platform/gtk/ValidationBubbleGtk.cpp:83
>> +    gtk_popover_set_position(GTK_POPOVER(m_popover), GTK_POS_TOP);
> 
> Is it needed to move the “gtk_popover_set_position()“ call? Also, maybe it would be
> possible to move the call after the “#endif” given that it is the same for both GTK3
> and GTK4.

I was concerned about flickering possibly caused by the gtk_popover_set_position call after attaching the popover to parents. But it seems fine. I have moved this line below #endif.

[Adrian Perez]

Comment on attachment 449426 [details]
Patch

Patch LGTM now, thanks for the update!

[EWS]

Committed r288156 (246150@main): <https://commits.webkit.org/246150@main>

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