Bug 235303 - [GTK] Implement native form validation bubbles for GTK4
Summary: [GTK] Implement native form validation bubbles for GTK4
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: ChangSeok Oh
URL:
Keywords:
Depends on:
Blocks: 167579
  Show dependency treegraph
 
Reported: 2022-01-17 14:48 PST by ChangSeok Oh
Modified: 2022-01-18 15:58 PST (History)
4 users (show)

See Also:


Attachments
Patch (5.62 KB, patch)
2022-01-17 19:52 PST, ChangSeok Oh
cgarcia: review+
cgarcia: commit-queue-
Details | Formatted Diff | Diff
Patch (5.32 KB, patch)
2022-01-18 14:42 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ChangSeok Oh 2022-01-17 14:48:31 PST
This is a follow-up ticket for bug 234629. We enable native validation bubbles for gtk4.
Comment 1 ChangSeok Oh 2022-01-17 19:52:07 PST
Created attachment 449362 [details]
Patch
Comment 2 Carlos Garcia Campos 2022-01-18 00:25:35 PST
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.
Comment 3 Adrian Perez 2022-01-18 00:28:29 PST
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.
Comment 4 ChangSeok Oh 2022-01-18 14:42:00 PST
Created attachment 449426 [details]
Patch
Comment 5 ChangSeok Oh 2022-01-18 14:50:06 PST
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.
Comment 6 Adrian Perez 2022-01-18 15:19:20 PST
Comment on attachment 449426 [details]
Patch

Patch LGTM now, thanks for the update!
Comment 7 EWS 2022-01-18 15:57:57 PST
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].