On fast-clicks/double-clicks on a plugin row the selected row can change and the buttons (e.g. the "Help" button) can get out of sync. That means that e.g. a plugin is selected which has not got an help function but the "Help" button is activated. Clicking the button is such a situation leads to a crash. Syncing buttons on "button-release-event" solves the problem. Fixes #1781. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1784
-- Commit Summary --
* plugin-manager: sync buttons on "button-release-event"
-- File Changes --
M src/plugins.c (19)
-- Patch Links --
https://github.com/geany/geany/pull/1784.patch https://github.com/geany/geany/pull/1784.diff
I think the workaround is acceptable. @elextr @b4n @codebrainz any objections?
LGBI
I havn't had a chance to test it, but yeah, in the face of a GTK bug a workaround is needed because even if the problem is fixed there are buggy versions in the wild already.
I haven't thoroughly reviewed or tested the code, but I have absolutely no objection over merging it nice and early in the release cycle.
I think that first of all we should fix the code so that it doesn't only rely on the UI being insensitive not to crash. At least, do something like this: ```diff diff --git a/src/plugins.c b/src/plugins.c index 32a0ee891..c036ea0dd 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -1865,7 +1865,10 @@ static void pm_on_plugin_button_clicked(G_GNUC_UNUSED GtkButton *button, gpointe if (GPOINTER_TO_INT(user_data) == PM_BUTTON_CONFIGURE) plugin_show_configure(&p->public); else if (GPOINTER_TO_INT(user_data) == PM_BUTTON_HELP) + { + g_return_if_fail(p->cbs.help != NULL); p->cbs.help(&p->public, p->cb_data); + } else if (GPOINTER_TO_INT(user_data) == PM_BUTTON_KEYBINDINGS && p->key_group && p->key_group->plugin_key_count > 0) keybindings_dialog_show_prefs_scroll(p->info.name); } ```
Then, I'll try and play with this to see if we're not doing something wrong somewhere, because it looks like a very odd bug, and it only happens when double-clicking on the checkmark. Also, we really should be using the treeview's cursor rather than the selection, but well.
I think that first of all we should fix the code so that it doesn't only rely on the UI being insensitive not to crash.
Yes, both keybindings and preferences buttons don't seem to suffer the problem, probably because of a similar test.
@LarsGit223 pushed 1 commit.
9e49575 plugin-manager: protect call to help function with NULL pointer check
Also, we really should be using the treeview's cursor rather than the selection, but well.
I haven't used cursors yet. How should it be used? Somewhat like this: - on clicking a checkbox in the pluing manager set the cursor to the path given to ```pm_plugin_toggled()``` - in ```pm_on_plugin_button_clicked``` get the current cursor instead of the selection to determine which row/plugin is active/to be called
Do we need to track ```cursor-changed```? Can the user change the cursor position if we do not have any editable cells?
@LarsGit223 basically when you play with the selection as a matter of having the "current element", using the cursor instead is the basically same and IMO makes more sense as the selection can possibly allow multiple rows and such. But anyway, it doesn't really matter, I just like it better, it shouldn't change anything -- and I tested, and it doesn't magically fix the issue.
However, I did one experiment: if I remove the GtkTreeModelFilter, the bug disappears. Obviously we can't filter anymore, but the selection/cursor doesn't jump around anymore. I'll try and investigate whether it's a mistake on our end (like confusing filtered and unfiltered stores, although I kind of doubt it as it happens when the filter doesn't do anything), or if it's indeed a bug in there. Unless someone else beats me to it ;)
@b4n: thanks for the info. I also played around with the cursor a bit. I managed to use it as a replacement but also noticed that it does not make any difference. Setting selection mode to none also does not help.
Obviously we can't filter anymore, but the selection/cursor doesn't jump around anymore.
Interesting. Will have a look too.
OK I found the root cause of the issue, which actually is a problem on our side. See #1799 for details and a fix.
Ok, thanks.
Closed #1784.
github-comments@lists.geany.org