Bug 235303

Summary: [GTK] Implement native form validation bubbles for GTK4
Product: WebKit Reporter: ChangSeok Oh <changseok>
Component: WebKitGTKAssignee: ChangSeok Oh <changseok>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, aperez, bugs-noreply, cgarcia
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 167579    
Attachments:
Description Flags
Patch
cgarcia: review+, cgarcia: commit-queue-
Patch none

ChangSeok Oh
Reported 2022-01-17 14:48:31 PST
This is a follow-up ticket for bug 234629. We enable native validation bubbles for gtk4.
Attachments
Patch (5.62 KB, patch)
2022-01-17 19:52 PST, ChangSeok Oh 		cgarcia: review+
cgarcia: commit-queue-
DetailsFormatted DiffDiff
Patch (5.32 KB, patch)
2022-01-18 14:42 PST, ChangSeok Oh 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
ChangSeok Oh
Comment 1 2022-01-17 19:52:07 PST
Created attachment 449362 [details] Patch
Carlos Garcia Campos
Comment 2 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.
Adrian Perez
Comment 3 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.
ChangSeok Oh
Comment 4 2022-01-18 14:42:00 PST
Created attachment 449426 [details] Patch
ChangSeok Oh
Comment 5 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.
Adrian Perez
Comment 6 2022-01-18 15:19:20 PST
Comment on attachment 449426 [details] Patch Patch LGTM now, thanks for the update!
EWS
Comment 7 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].
Note You need to log in before you can comment on or make changes to this bug.