GNOME Bugzilla – Bug 751345
[PATCH] Using tab group cause plugin add another copy of entry to the menu
Last modified: 2019-03-23 20:33:53 UTC
irc log: 2015.05.30 ____ [03:52:13] asl97: does anyone know if the bug where using tab group cause plugin like join/split line and code comment to add another copy of entry to the menu has been fix? [03:52:41] asl97: image is set to be deleted in an hour, i can reupload it if needed: [image been deleted long ago] [03:57:55] gregier: asl97: not fixed [03:59:18] gregier: and afaik it is a new bug, we probably don't use the context menu that much so it just went unnoticed [04:02:05] gregier: although it looks like it must be a gedit/gsv/gtk+ issue [04:07:00] asl97: i use the context menu quite a bit but this is the first time i saw it cause i accidentally click the left mouse button then the right only to see the group tab. i played with it and before i know it, the menu became long [04:07:16] gregier: yeah very weird, the plugins just connect to a signal and add menu items [04:07:41] gregier: so I'm guessing that the signal is being called multiple times for some reason [04:22:22] gregier: freaky! left tab group has 2x, right tab group has 3x [07:35:26] asl97: gregier: should i file the bug or will you be filing it? [13:56:29] gregier: asl97: best if you file it
Created attachment 348479 [details] [review] Fix popup item duplication.patch
The bug affect the joinlines and codecomment plugin When enabling it, it add an handler to the `populate-popup`. But disabling it doesn't remove it. So when disabling and re-enabling it, an additional handler is added causing the duplication. Splitting the view seem to trigger the activate causing more additional handle to be added, resulting in different number of handler in each view, causing the `left tab group has 2x, right tab group has 3x`. see patch for fix.
Review of attachment 348479 [details] [review]: Looks good, but to have more robust code, the signal should be disconnected only if self.popup_handler_id != 0 (and then set it to 0). The class constructor should probably initialize self.popup_handler_id to 0, otherwise the instance variable doesn't always exist. Normally do_deactivate is called only once, and always after do_activate, but it's anyway safer to have more robust code, just in case.
Created attachment 353942 [details] [review] Fix popup item duplication
Meh, it's stupid how something like this is stalling the bugfix. IMO, those ain't needed but since I ain't gonna be maintaining it and it's ain't my code base, let's just do it your way and get this over with.
Thank you for helping to make gedit better. I adjusted the commit message and pushed for you.