| Summary: | [GTK4] Disable arrow on context menu popover | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alice Mikhaylenko <alicem> | ||||||
| Component: | WebKitGTK | Assignee: | Adrian Perez <aperez> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aperez, bugs-noreply, cgarcia, jmason, mcatanzaro | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=170553 https://bugs.webkit.org/show_bug.cgi?id=211557 |
||||||||
| Attachments: |
|
||||||||
(In reply to Alexander Mikhaylenko from comment #0) > Created attachment 398067 [details] > Screenshot > > GTK doesn't do it for context menus in GtkEntry/GtkTextView, see screenshot. > > It's debatable what's the "right" behavior on GTK3 though, as it's all over > the place. I suppose we can just disable the arrow to make the context menus look like the rest of context menus in GTK4, and I would disable it for both versions for the sake of coherence (and avoiding #ifdefs in the code). (In reply to Adrian Perez from comment #1) > (In reply to Alexander Mikhaylenko from comment #0) > > Created attachment 398067 [details] > > Screenshot > > > > GTK doesn't do it for context menus in GtkEntry/GtkTextView, see screenshot. > > > > It's debatable what's the "right" behavior on GTK3 though, as it's all over > > the place. > > I suppose we can just disable the arrow to make the context menus look like > the rest of context menus in GTK4, and I would disable it for both versions > for the sake of coherence (and avoiding #ifdefs in the code). Actually, now that I checked it seems that there is no API in GTK3 to disable the popover arrows and they are unconditionally drawn. For GTK4 we can use gtk_popover_set_has_arrow() to disable them. Oh well, then GTK4 only. Created attachment 398105 [details]
Patch
(In reply to Alexander Mikhaylenko from comment #3) > Oh well, then GTK4 only. I have filed an issue requesting the gtk_popover_{get,set}_has_arrow() for GTK3: https://gitlab.gnome.org/GNOME/gtk/-/issues/2679 — it would be neat to have, but I guess it's not a huge deal. Committed r260986: <https://trac.webkit.org/changeset/260986> All reviewed patches have been landed. Closing bug and clearing flags on attachment 398105 [details]. We should probably only use this API in GTK 4, since it doesn't do what we want in GTK 3. (In reply to Michael Catanzaro from comment #7) > We should probably only use this API in GTK 4, since it doesn't do what we > want in GTK 3. It doesn't even exist in GTK3. Have you seen the stub in GtkVersioning.h? I mean: let's not use an API that makes an arrow appear. (In reply to Michael Catanzaro from comment #9) > I mean: let's not use an API that makes an arrow appear. Maybe I am not explaining myself properly: - GTK3: We cannot control visibility of the arrow (it's always painted). - GTK4: There is gtk_popover_set_has_arrow(), used to disable it. - In GtkVersioning.h, an empty gtk_popover_set_has_arrow() is added *only used for GTK3* so the code in WebContextMenuProxyGtk.cpp does not need #ifdefs. So, no, an arrow is never made to appear. On the contrary: it's always there by default, and on GTK4 we make it go away 🤷️ Look, in 2.28 there was no arrow on the context menu. Now going forward there is going to be an arrow. Do we really want to deal with complaints from users about this? It looks silly. (In reply to Michael Catanzaro from comment #11) > Look, in 2.28 there was no arrow on the context menu. Now going forward > there is going to be an arrow. Do we really want to deal with complaints > from users about this? It looks silly. We don't want to maintain different code paths for GTK3 and GTK4. If you want my personal opinion, adding gtk_popover_set_has_arrow() to GTK3 does not look too complicated, but my proposal for that has been shot down already: https://gitlab.gnome.org/GNOME/gtk/-/issues/2679 Popover is going to look different from a GtkMenu in any case, so not a big deal whether it has arrow in GTK3 really. GTK4 is another story because it itself does context menus as popovers with no arrow, like the patch does. (In reply to Adrian Perez from comment #12) > (In reply to Michael Catanzaro from comment #11) > > Look, in 2.28 there was no arrow on the context menu. Now going forward > > there is going to be an arrow. Do we really want to deal with complaints > > from users about this? It looks silly. > > We don't want to maintain different code paths for GTK3 and GTK4. If you > want my personal opinion, adding gtk_popover_set_has_arrow() to GTK3 does > not look too complicated, but my proposal for that has been shot down > already: https://gitlab.gnome.org/GNOME/gtk/-/issues/2679 Even if the proposal had been accepted, I have a doubt. Maybe not everyone wants to or is able to upgrade to the latest gtk3 release to pick up the backported function to compile or run webkit. Also, in addition to the visual hackishness of the nipple artefact, the new context menu implementation breaks mouse click semantics. I've opened Bug 211294 to track that. (In reply to Jim Mason from comment #14) > (In reply to Adrian Perez from comment #12) > > (In reply to Michael Catanzaro from comment #11) > > > Look, in 2.28 there was no arrow on the context menu. Now going forward > > > there is going to be an arrow. Do we really want to deal with complaints > > > from users about this? It looks silly. > > > > We don't want to maintain different code paths for GTK3 and GTK4. If you > > want my personal opinion, adding gtk_popover_set_has_arrow() to GTK3 does > > not look too complicated, but my proposal for that has been shot down > > already: https://gitlab.gnome.org/GNOME/gtk/-/issues/2679 > > Even if the proposal had been accepted, I have a doubt. Maybe not everyone > wants to or is able to upgrade to the latest gtk3 release to pick up the > backported function to compile or run webkit. > > Also, in addition to the visual hackishness of the nipple artefact, the new > context menu implementation breaks mouse click semantics. I've opened Bug > 211294 to track that. After some discussion with Carlos García, I have opened bug #211557 to bring back the GtkMenuShell-based context menus for GTK3. That will fix the regressions and get rid of the arrow. Note that GTK4 will keep using the GtkPopoverMenu, so if there are context menu behaviors which would be desirable to keep (like selecting items with the right button), then those should be reported as bugs in the GTK project, at https://gitlab.gnome.org/GNOME/gtk/-/issues I hope doing things in this way will work out for the best 🙂️ |
Created attachment 398067 [details] Screenshot GTK doesn't do it for context menus in GtkEntry/GtkTextView, see screenshot. It's debatable what's the "right" behavior on GTK3 though, as it's all over the place.