Summary: | [GTK] Implement native form validation bubbles for GTK4 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | ChangSeok Oh <changseok> | ||||||
Component: | WebKitGTK | Assignee: | 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
ChangSeok Oh
2022-01-17 14:48:31 PST
Created attachment 449362 [details]
Patch
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 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. Created attachment 449426 [details]
Patch
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 on attachment 449426 [details]
Patch
Patch LGTM now, thanks for the update!
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]. |