From 77fc6e35397d96c4a511d65dff58ce42e58a39ec Mon Sep 17 00:00:00 2001 From: Kristian Rietveld Date: Wed, 30 Sep 2009 10:19:07 +0200 Subject: [PATCH] Bug 596580 - Blank rows in entry autocompletion gtk_tree_model_build_level() always needs to emit row-inserted when requested, this should not depend on whether the level has a parent level or a virtual root, which is a check whether or not we need to reference the node in the child model. Furthermore, we also need to emit row-has-child-toggled after row-inserted when appropriate. When gtk_tree_model_filter_row_changed() pulls in the root level, it must request build_level() to emit signals for this. The refilter function uses row_changed to process the changes, so build_level() in the first call to row_changed() might pull in multiple new nodes in this scenario, for all of these signals need to be emitted. Of course, build_level() will then also emit the signals for the node row_changed() is processing, we should not emit a duplicate signal, this is now accounted for. Add a unit test for this. For this small functionality to block the row-changed signal has been implemented, so that we can simulate calls to the refilter function using the current visible column setup. --- gtk/gtktreemodelfilter.c | 51 +++++---- gtk/tests/filtermodel.c | 227 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 259 insertions(+), 19 deletions(-) diff --git a/gtk/gtktreemodelfilter.c b/gtk/gtktreemodelfilter.c index 09a8d4482f..074a7a4fa1 100644 --- a/gtk/gtktreemodelfilter.c +++ b/gtk/gtktreemodelfilter.c @@ -534,6 +534,7 @@ gtk_tree_model_filter_build_level (GtkTreeModelFilter *filter, { if (gtk_tree_model_filter_visible (filter, &iter)) { + GtkTreeIter f_iter; FilterElt filter_elt; filter_elt.offset = i; @@ -548,26 +549,29 @@ gtk_tree_model_filter_build_level (GtkTreeModelFilter *filter, g_array_append_val (new_level->array, filter_elt); new_level->visible_nodes++; + f_iter.stamp = filter->priv->stamp; + f_iter.user_data = new_level; + f_iter.user_data2 = &(g_array_index (new_level->array, FilterElt, new_level->array->len - 1)); + if (new_level->parent_level || filter->priv->virtual_root) + gtk_tree_model_filter_ref_node (GTK_TREE_MODEL (filter), &f_iter); + + if (emit_inserted) { - GtkTreeIter f_iter; + GtkTreePath *f_path; + GtkTreeIter children; - f_iter.stamp = filter->priv->stamp; - f_iter.user_data = new_level; - f_iter.user_data2 = &(g_array_index (new_level->array, FilterElt, new_level->array->len - 1)); + f_path = gtk_tree_model_get_path (GTK_TREE_MODEL (filter), + &f_iter); + gtk_tree_model_row_inserted (GTK_TREE_MODEL (filter), + f_path, &f_iter); + gtk_tree_path_free (f_path); - gtk_tree_model_filter_ref_node (GTK_TREE_MODEL (filter), &f_iter); - - if (emit_inserted) - { - GtkTreePath *f_path; - - f_path = gtk_tree_model_get_path (GTK_TREE_MODEL (filter), - &f_iter); - gtk_tree_model_row_inserted (GTK_TREE_MODEL (filter), - f_path, &f_iter); - gtk_tree_path_free (f_path); - } + if (gtk_tree_model_iter_children (filter->priv->child_model, + &children, &iter)) + gtk_tree_model_filter_update_children (filter, + new_level, + FILTER_ELT (f_iter.user_data2)); } } i++; @@ -1224,6 +1228,7 @@ gtk_tree_model_filter_row_changed (GtkTreeModel *c_model, gboolean requested_state; gboolean current_state; gboolean free_c_path = FALSE; + gboolean signals_emitted = FALSE; g_return_if_fail (c_path != NULL || c_iter != NULL); @@ -1310,7 +1315,13 @@ gtk_tree_model_filter_row_changed (GtkTreeModel *c_model, { FilterLevel *root; - gtk_tree_model_filter_build_level (filter, NULL, -1, FALSE); + gtk_tree_model_filter_build_level (filter, NULL, -1, TRUE); + + /* We will only proceed below if the item is found. If the item + * is found, we can be sure row-inserted has just been emitted + * for it. + */ + signals_emitted = TRUE; root = FILTER_LEVEL (filter->priv->root); } @@ -1349,7 +1360,8 @@ gtk_tree_model_filter_row_changed (GtkTreeModel *c_model, gtk_tree_path_free (path); path = gtk_tree_model_get_path (GTK_TREE_MODEL (filter), &iter); - gtk_tree_model_row_inserted (GTK_TREE_MODEL (filter), path, &iter); + if (!signals_emitted) + gtk_tree_model_row_inserted (GTK_TREE_MODEL (filter), path, &iter); if (level->parent_level && level->visible_nodes == 1) { @@ -1366,7 +1378,8 @@ gtk_tree_model_filter_row_changed (GtkTreeModel *c_model, &iter); } - if (gtk_tree_model_iter_children (c_model, &children, c_iter)) + if (!signals_emitted + && gtk_tree_model_iter_children (c_model, &children, c_iter)) gtk_tree_model_filter_update_children (filter, level, elt); } diff --git a/gtk/tests/filtermodel.c b/gtk/tests/filtermodel.c index ecc0db2a4d..eebd382038 100644 --- a/gtk/tests/filtermodel.c +++ b/gtk/tests/filtermodel.c @@ -367,8 +367,19 @@ typedef struct GtkTreeModelFilter *filter; SignalMonitor *monitor; + + guint block_signals : 1; } FilterTest; + +static void +filter_test_store_signal (FilterTest *fixture) +{ + if (fixture->block_signals) + g_signal_stop_emission_by_name (fixture->store, "row-changed"); +} + + static void filter_test_setup_generic (FilterTest *fixture, gconstpointer test_data, @@ -381,6 +392,9 @@ filter_test_setup_generic (FilterTest *fixture, fixture->store = create_tree_store (depth, !empty); + g_signal_connect_swapped (fixture->store, "row-changed", + G_CALLBACK (filter_test_store_signal), fixture); + /* Please forgive me for casting const away. */ filter = gtk_tree_model_filter_new (GTK_TREE_MODEL (fixture->store), (GtkTreePath *)vroot); @@ -628,6 +642,18 @@ filter_test_enable_filter (FilterTest *fixture) gtk_tree_model_filter_refilter (fixture->filter); } +static void +filter_test_block_signals (FilterTest *fixture) +{ + fixture->block_signals = TRUE; +} + +static void +filter_test_unblock_signals (FilterTest *fixture) +{ + fixture->block_signals = FALSE; +} + static void filter_test_teardown (FilterTest *fixture, gconstpointer test_data) @@ -1177,6 +1203,86 @@ empty_show_nodes (FilterTest *fixture, check_level_length (fixture->filter, "0:0:0", 0); } +static void +empty_show_multiple_nodes (FilterTest *fixture, + gconstpointer user_data) +{ + GtkTreeIter iter; + GtkTreePath *changed_path; + + check_filter_model (fixture); + check_level_length (fixture->filter, NULL, 0); + + signal_monitor_append_signal (fixture->monitor, ROW_INSERTED, "0"); + signal_monitor_append_signal (fixture->monitor, ROW_HAS_CHILD_TOGGLED, "0"); + signal_monitor_append_signal (fixture->monitor, ROW_INSERTED, "1"); + signal_monitor_append_signal (fixture->monitor, ROW_HAS_CHILD_TOGGLED, "1"); + signal_monitor_append_signal (fixture->monitor, ROW_CHANGED, "1"); + signal_monitor_append_signal (fixture->monitor, ROW_HAS_CHILD_TOGGLED, "1"); + + /* We simulate a change in visible func condition with this. The + * visibility state of multiple nodes changes at once, we emit row-changed + * for these nodes (and others) after that. + */ + filter_test_block_signals (fixture); + set_path_visibility (fixture, "3", TRUE); + set_path_visibility (fixture, "4", TRUE); + filter_test_unblock_signals (fixture); + + changed_path = gtk_tree_path_new (); + gtk_tree_path_append_index (changed_path, 2); + gtk_tree_model_get_iter (GTK_TREE_MODEL (fixture->store), + &iter, changed_path); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_next (changed_path); + gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store), &iter); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_next (changed_path); + gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store), &iter); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_free (changed_path); + + check_filter_model (fixture); + check_level_length (fixture->filter, NULL, 2); + check_level_length (fixture->filter, "0", 0); + + set_path_visibility (fixture, "3:2:2", TRUE); + check_filter_model (fixture); + check_level_length (fixture->filter, NULL, 2); + check_level_length (fixture->filter, "0", 0); + + signal_monitor_append_signal (fixture->monitor, ROW_INSERTED, "0:0"); + signal_monitor_append_signal (fixture->monitor, ROW_HAS_CHILD_TOGGLED, "0"); + signal_monitor_append_signal (fixture->monitor, ROW_HAS_CHILD_TOGGLED, "0:0"); + set_path_visibility (fixture, "3:2", TRUE); + check_filter_model (fixture); + check_level_length (fixture->filter, NULL, 2); + check_level_length (fixture->filter, "0", 1); + check_level_length (fixture->filter, "0:0", 1); + check_level_length (fixture->filter, "0:0:0", 0); + + signal_monitor_append_signal (fixture->monitor, ROW_DELETED, "0"); + set_path_visibility (fixture, "3", FALSE); + check_filter_model (fixture); + check_level_length (fixture->filter, NULL, 1); + + signal_monitor_append_signal (fixture->monitor, ROW_INSERTED, "0"); + signal_monitor_append_signal (fixture->monitor, ROW_HAS_CHILD_TOGGLED, "0"); + set_path_visibility (fixture, "3:2:1", TRUE); + set_path_visibility (fixture, "3", TRUE); + check_filter_model (fixture); + check_level_length (fixture->filter, NULL, 2); + check_level_length (fixture->filter, "0", 1); + check_level_length (fixture->filter, "0:0", 2); + check_level_length (fixture->filter, "0:0:0", 0); +} + static void empty_vroot_show_nodes (FilterTest *fixture, gconstpointer user_data) @@ -1221,6 +1327,117 @@ empty_vroot_show_nodes (FilterTest *fixture, check_level_length (fixture->filter, "0:1", 0); } +static void +empty_vroot_show_multiple_nodes (FilterTest *fixture, + gconstpointer user_data) +{ + GtkTreeIter iter; + GtkTreePath *changed_path; + GtkTreePath *path = (GtkTreePath *)user_data; + + check_filter_model_with_root (fixture, path); + check_level_length (fixture->filter, NULL, 0); + + /* We simulate a change in visible func condition with this. The + * visibility state of multiple nodes changes at once, we emit row-changed + * for these nodes (and others) after that. + */ + filter_test_block_signals (fixture); + set_path_visibility (fixture, "2", TRUE); + set_path_visibility (fixture, "3", TRUE); + filter_test_unblock_signals (fixture); + + changed_path = gtk_tree_path_new (); + gtk_tree_path_append_index (changed_path, 1); + gtk_tree_model_get_iter (GTK_TREE_MODEL (fixture->store), + &iter, changed_path); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_next (changed_path); + gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store), &iter); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_next (changed_path); + gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store), &iter); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_next (changed_path); + gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store), &iter); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_free (changed_path); + + check_filter_model_with_root (fixture, path); + check_level_length (fixture->filter, NULL, 0); + + set_path_visibility (fixture, "2:2:2", TRUE); + check_filter_model_with_root (fixture, path); + check_level_length (fixture->filter, NULL, 0); + + signal_monitor_append_signal (fixture->monitor, ROW_INSERTED, "0"); + signal_monitor_append_signal (fixture->monitor, ROW_HAS_CHILD_TOGGLED, "0"); + signal_monitor_append_signal (fixture->monitor, ROW_INSERTED, "1"); + signal_monitor_append_signal (fixture->monitor, ROW_HAS_CHILD_TOGGLED, "1"); + + /* Again, we simulate a call to refilter */ + filter_test_block_signals (fixture); + set_path_visibility (fixture, "2:2", TRUE); + set_path_visibility (fixture, "2:3", TRUE); + filter_test_unblock_signals (fixture); + + changed_path = gtk_tree_path_new (); + gtk_tree_path_append_index (changed_path, 2); + gtk_tree_path_append_index (changed_path, 1); + gtk_tree_model_get_iter (GTK_TREE_MODEL (fixture->store), + &iter, changed_path); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_next (changed_path); + gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store), &iter); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_next (changed_path); + gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store), &iter); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_next (changed_path); + gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store), &iter); + gtk_tree_model_row_changed (GTK_TREE_MODEL (fixture->store), + changed_path, &iter); + + gtk_tree_path_free (changed_path); + + check_filter_model_with_root (fixture, path); + check_level_length (fixture->filter, NULL, 2); + check_level_length (fixture->filter, "0", 1); + check_level_length (fixture->filter, "0:0", 0); + + set_path_visibility (fixture, "3", TRUE); + check_filter_model_with_root (fixture, path); + check_level_length (fixture->filter, NULL, 2); + + signal_monitor_append_signal (fixture->monitor, ROW_DELETED, "0"); + set_path_visibility (fixture, "2:2", FALSE); + check_filter_model_with_root (fixture, path); + check_level_length (fixture->filter, NULL, 1); + + signal_monitor_append_signal (fixture->monitor, ROW_INSERTED, "0"); + signal_monitor_append_signal (fixture->monitor, ROW_HAS_CHILD_TOGGLED, "0"); + set_path_visibility (fixture, "2:2:1", TRUE); + set_path_visibility (fixture, "2:2", TRUE); + check_filter_model_with_root (fixture, path); + check_level_length (fixture->filter, NULL, 2); + check_level_length (fixture->filter, "0", 2); + check_level_length (fixture->filter, "0:1", 0); +} + static void unfiltered_hide_single (FilterTest *fixture, @@ -2659,12 +2876,22 @@ main (int argc, filter_test_setup_empty, empty_show_nodes, filter_test_teardown); + g_test_add ("/FilterModel/empty/show-multiple-nodes", + FilterTest, NULL, + filter_test_setup_empty, + empty_show_multiple_nodes, + filter_test_teardown); g_test_add ("/FilterModel/empty/show-nodes/vroot", FilterTest, gtk_tree_path_new_from_indices (2, -1), filter_test_setup_empty, empty_vroot_show_nodes, filter_test_teardown); + g_test_add ("/FilterModel/empty/show-multiple-nodes/vroot", + FilterTest, gtk_tree_path_new_from_indices (2, -1), + filter_test_setup_empty, + empty_vroot_show_multiple_nodes, + filter_test_teardown); g_test_add ("/FilterModel/unfiltered/hide-single",