Bug 211241 – [GTK4] Disable arrow on context menu popover

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

[Alice Mikhaylenko]

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.

[Adrian Perez]

(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).

[Adrian Perez]

(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.

[Alice Mikhaylenko]

Oh well, then GTK4 only.

[Adrian Perez]

Created attachment 398105 [details]
Patch

[Adrian Perez]

(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.

[EWS]

Committed r260986: <https://trac.webkit.org/changeset/260986>

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

[Michael Catanzaro]

We should probably only use this API in GTK 4, since it doesn't do what we want in GTK 3.

[Adrian Perez]

(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?

[Michael Catanzaro]

I mean: let's not use an API that makes an arrow appear.

[Adrian Perez]

(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 🤷️

[Michael Catanzaro]

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.

[Adrian Perez]

(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

[Alice Mikhaylenko]

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.

[Jim Mason]

(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.

[Adrian Perez]

(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 🙂️