GNOME Bugzilla – Bug 776949
FileChooserButton: seg fault when clicked
Last modified: 2017-08-27 13:01:35 UTC
When clicking on the FileChooserButton (gtk 3.22.6) with SELECT_FOLDER action, I get a seg fault at gtk+/gtl/gtkmenushell.c:1249: g_return_if_fail (GTK_IS_MENU_ITEM (menu_item)); I managed to avoid it by setting a uri for the FileChooserButton before using it, but it's a little inconvenient.
This is caused by the fix for bug 771242. Opening the menu item causes filechooserbutton to refilter the treemodel which causes the treemenu to redo its menuitems, so the menuitem stored by the combobox is no longer existing to be selected afterwards. Oh yeah, and I hereby scold you Visarion, because I told you this on IRC but you didn't include any of this here in the bug report.
https://git.gnome.org/browse/gtk+/tree/gtk/gtkcombobox.c#n1758 This seems really circular: * We traverse the active and visible items, to get the desired rect-anchor-dy, to center the item that will be selected on popup * We use this rect-anchor-dy to open the menu * Opening the menu cause the model to be redone, * so the rect-anchor-dy may not be valid anymore... if it ever was. Moreover, it seems this is done even if there isn't an active item... so why are we offsetting? I'm preparing a patch that doesn't do this. It doesn't solve this issue but seems to make more sense. The whole thing really doesn't work if the act of popping up the menu causes its items to be refiltered, though. Is that a good idea? Which of ComboBox and FileChooserButton is being less dodgy in this case?
Created attachment 343198 [details] [review] [PATCH 1/2] combobox: Move some variables into narrowest scope Add git squashing to taste.
Created attachment 343199 [details] [review] [PATCH 2/2] combobox: Work around popup handler altering model I don't know whether this is a nice fix, but then it's hard to imagine how one of _those_ would be feasible here... Anyway, it's a first try. Hopefully I've not made anything _worse_, at least! (but look how that worked out last time) > GtkFileChooserButton installs a handler for the popped-up signal, which > refilters the menu, in order to hide the "(None)" item from the popup > if it was previous selected and visible in the ComboBox. This meant that > > * Until not too long ago, this item would be selected in the menu > shell, which would then be popped up and change from that selection. > This was therefore redundant (more on which below!) but benign. > > * After the patch for https://bugzilla.gnome.org/show_bug.cgi?id=771242 > however, this causes a critical assertion fail, as now we stash the > originally selected item in a pointer so that it can be selected only > after realisation/popup - but by that stage, the model has just been > refiltered and the previous pointer no longer refers to a valid item. > > This commit works around this problem by, after popping up the menu, > getting the active item again, in case a popped-up handler has > invalidated the pointer to the active item that we got before popup. > > Sadly, this means that in such cases, the loop getting the y-offset of > the active item, to centre it above the ComboBox, is a waste of time. > This is desirable for CBs who don't alter their model in popped-up > (which one hopes are the majority - and it's difficult to think of a way > to check if the active item is /going to/ be hidden so as to avoid this.
Created attachment 343200 [details] [review] [PATCH 2/2] combobox: Work around popup handler altering model fixed up typos in the (voluminous) commit message
Benjamin said on IRC to go for it, so I've committed the attached patches with minor tweaks (and missing link back here, d'oh...) as: gtk-3-22: e98e6f73be8ec7647d6eddeacf6d13cd96352589 combobox: Work around popup handler altering model dfe89a381fbf6648228aaaff198f20f620b084bc combobox: Don’t select active item if it’s hidden ac4e1625f5f3d806456ad5d75d7dd0d8b24d8164 combobox: Move variables into narrowest scopes master: e4ede33a650d91804327f052c76322ff06d26a95 combobox: Work around popup handler altering model 7a5c995fd4a5da18fafbdecee7cd151766e8b00b combobox: Don’t select active item if it’s hidden 2e973cedc9793b15c18e142c7564510c27790cb2 combobox: Move variables into narrowest scopes