combobox: Work around popup handler altering model

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 previously selected in the ComboBox. This oddity means that:

 • Until recently, this item would be selected in the menu shell, which
   would then be popped up and change the selection away from that item.
   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 gone and
invalidated the pointer to the active item that we saved before popup.

If a handler does this, everything done to find/use the original item is
pointless. But this avoids the ugly critical in FileChooserButton, while
not harming every other ComboBox that doesn’t mess with its model while
popping up (hopefully the vast majority), and it’s very difficult to
imagine a way to check if the active item is /going to/ be hidden later)

1 files changed, 7 insertions, 3 deletions

diff --git a/gtk/gtkcombobox.c b/gtk/gtkcombobox.c
index 4b3c9f8703..8173876d36 100644
--- a/gtk/gtkcombobox.c
+++ b/gtk/gtkcombobox.c
@@ -1755,7 +1755,6 @@ gtk_combo_box_menu_popup (GtkComboBox    *combo_box,
     {
       /* FIXME handle nested menus better */
       GtkWidget *active = gtk_menu_get_active (GTK_MENU (priv->popup_widget));;
-      GtkWidget *select = active;
       gint rect_anchor_dy = -2;
       GList *i;
       GtkWidget *child;
@@ -1807,8 +1806,13 @@ gtk_combo_box_menu_popup (GtkComboBox    *combo_box,
                                 GDK_GRAVITY_NORTH_WEST,
                                 trigger_event);
 
-      if (select)
-        gtk_menu_shell_select_item (GTK_MENU_SHELL (priv->popup_widget), select);
+      /* As a hack, re-get the active item, in case a popup handler, like that
+       * of FileChooserButton, just caused the menu to be refiltered, making the
+       * previous active item pointer invalid now. This seems pretty ugly and
+       * makes the y-offset loop pointless for such cases, so FIXME later? */
+      active = gtk_menu_get_active (GTK_MENU (priv->popup_widget));
+      if (active)
+        gtk_menu_shell_select_item (GTK_MENU_SHELL (priv->popup_widget), active);
     }
 }