Fix context menus on walyand (see #3009). You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3011
-- Commit Summary --
* fix context menus on wayland (#3009)
-- File Changes --
M src/callbacks.c (4) M src/editor.c (5) M src/msgwindow.c (12) M src/notebook.c (4) M src/plugins.c (4) M src/prefs.c (4) M src/sidebar.c (8) M src/symbols.c (4) M src/vte.c (4)
-- Patch Links --
https://github.com/geany/geany/pull/3011.patch https://github.com/geany/geany/pull/3011.diff
@dolik-rce pushed 1 commit.
b1738f0f5a117ec1accb773b0437e40f723d3670 fix context menus on wayland (#3009)
@dolik-rce pushed 1 commit.
85de03686c39993d11adab0ef87aaed2dd97b186 fix context menus on wayland (#3009)
@eht16 commented on this pull request.
@@ -634,7 +634,11 @@ gboolean toolbar_popup_menu(GtkWidget *widget, GdkEventButton *event, gpointer u
{ if (event->button == 3) { +#if GTK_CHECK_VERSION(3,22,0)
The GTK version check and popup menu commands repeat quite often and look very similar.
Could we maybe put them into a helper function, maybe `ui_menu_poopup()` in "ui_utils.c" or so, to not have to repeat this everywhere? Also we could add a sentence or two to document what happens there.
Looks good. I'll test it in the next days on a non-Wayland and a Windows system to ensure it doesn't break there.
@elextr commented on this pull request.
@@ -634,7 +634,11 @@ gboolean toolbar_popup_menu(GtkWidget *widget, GdkEventButton *event, gpointer u
{ if (event->button == 3) { +#if GTK_CHECK_VERSION(3,22,0)
Can we just assume 3.22 IAW [this comment](https://github.com/geany/geany/pull/3178#issuecomment-1128174560)
@dolik-rce pushed 1 commit.
cce7794c5ef5150db058c440c4bf0b0833f000cd Refactor context menu code to ui_menu_popup
@dolik-rce commented on this pull request.
@@ -634,7 +634,11 @@ gboolean toolbar_popup_menu(GtkWidget *widget, GdkEventButton *event, gpointer u
{ if (event->button == 3) { +#if GTK_CHECK_VERSION(3,22,0)
@eht16: Sure, makes sense. Thanks for the hint on the function name and where it should be located, that was really helpful.
@elextr: It would definitely make things simpler, if there is agreement that 3.22 or higher is required. But I believe it is not yet enforced in autotools nor in meson, so I will keep it backward compatible, at least for now, ok?
@elextr commented on this pull request.
@@ -634,7 +634,11 @@ gboolean toolbar_popup_menu(GtkWidget *widget, GdkEventButton *event, gpointer u
{ if (event->button == 3) { +#if GTK_CHECK_VERSION(3,22,0)
so I will keep it backward compatible, at least for now, ok?
Sure, was just trying to see if it could be simplified :-)
@eht16 commented on this pull request.
@@ -2300,7 +2300,6 @@ static void ui_menu_move(GtkWidget *menu, GtkWidget *old, GtkWidget *new)
g_object_unref(menu); }
-
Any reason for removing this line?
Looks good. I'll test it in the next days on a non-Wayland and a Windows system to ensure it doesn't break there.
Tested on Debian Testing and Windows 7, all modified popup menus appear as expected. Good job!
@dolik-rce commented on this pull request.
@@ -2300,7 +2300,6 @@ static void ui_menu_move(GtkWidget *menu, GtkWidget *old, GtkWidget *new)
g_object_unref(menu); }
-
Just me being stupid and not checking the code carefully enough before committing. I'll put it back.
@dolik-rce pushed 1 commit.
3bffc4518de6d514de1b419fed36399b013e0301 Refactor context menu code to ui_menu_popup
@eht16 commented on this pull request.
@@ -2300,7 +2300,6 @@ static void ui_menu_move(GtkWidget *menu, GtkWidget *old, GtkWidget *new)
g_object_unref(menu); }
-
No worries, thanks for reverting it.
If there are no objections, I'd merge this in a week or so.
@eht16: Thanks for testing. The mistakenly removed line is back now.
Merged #3011 into master.
I'm late to the party, but…
Looks good. I'll test it in the next days on a non-Wayland and a Windows system to ensure it doesn't break there.
Tested on Debian Testing and Windows 7, all modified popup menus appear as expected. Good job!
…but the go to declaration/definition if triggered by a keyboard event (e.g. <kbd>Primary+t</kbd>). It's not terrible, but sad. And if the autocompletion popup work OK, we should be able to also adjust the logic we have to also work there (I guess being Wayland it would mean stopping to be relative to the X/Y root, but using more relative coordinates?). And if we don't, we should really just drop all idea of popup positioning function for those, as it's effectively only used in GTK < 3.22, which basically nobody will have nowadays.
You're right @b4n, I never noticed that. Guess I'm not using those shortcuts much. I'll look at it and try to send a PR with a fix.
@dolik-rce if you are at it and want to do it, you could even drop the `ui_menu_popup()` function and use the GTK 3.22+ variant directly as we plan to require 3.24 quite soon.
@b4n: I have it almost ready. Making it work with the old GTK was the worst part :slightly_smiling_face: But if you say it's OK to drop support for old versions, it will make everything simpler.
github-comments@lists.geany.org