[evolution/gnome-2-30] Bug 622535 - Account assistant loops on first run

[Matthew Barnes <mbarnes src gnome org>]

commit e85a3fe56f9e329088ebe0d1a51a2dc07b112658
Author: Matthew Barnes <mbarnes redhat com>
Date:   Fri Jun 25 17:56:47 2010 -0400

    Bug 622535 - Account assistant loops on first run
    
    The issue was EConfig's GtkAssistantPageFunc callback assumed the pages
    in the GtkAssistant were sorted, but that assumption breaks as EPlugins
    with custom EConfigItemFactoryFunc callbacks are introduced and EConfig
    has to rebuild its assistant pages.
    
    It's an unnecessary requirement anyway, since EConfig already keeps a
    sorted list of widgets internally.  After correcting that assumption a
    number of hacks addressing previous page ordering bugs fell out.

 e-util/e-config.c        |  160 ++++++++++++++++++++++++++++-----------------
 mail/em-account-editor.c |   28 +++------
 2 files changed, 108 insertions(+), 80 deletions(-)
---
diff --git a/e-util/e-config.c b/e-util/e-config.c
index 613971f..e2c772e 100644
--- a/e-util/e-config.c
+++ b/e-util/e-config.c
@@ -94,7 +94,7 @@ config_finalize (GObject *object)
 	EConfigPrivate *p = emp->priv;
 	GList *link;
 
-	d(printf("finalising EConfig %p\n", o));
+	d(printf("finalising EConfig %p\n", object));
 
 	g_free(emp->id);
 
@@ -381,54 +381,62 @@ ep_cmp(gconstpointer ap, gconstpointer bp)
 	return strcmp(a->item->path, b->item->path);
 }
 
-static struct _widget_node *
+static GList *
 ec_assistant_find_page (EConfig *ec, GtkWidget *page, gint *page_index)
 {
-	struct _widget_node *wn = NULL;
+	struct _widget_node *node = NULL;
 	GList *link;
 
 	g_return_val_if_fail (ec != NULL, NULL);
 	g_return_val_if_fail (GTK_IS_ASSISTANT (ec->widget), NULL);
 	g_return_val_if_fail (page != NULL, NULL);
 
-	link = ec->priv->widgets;
+	/* Assume failure, then if we do fail we can just return. */
+	if (page_index != NULL)
+		*page_index = -1;
 
-	while (link != NULL) {
-		wn = link->data;
+	/* Find the page widget in our sorted widget node list. */
+	for (link = ec->priv->widgets; link != NULL; link = link->next) {
+		node = link->data;
 
-		if (wn->frame == page
-		    && (wn->item->type == E_CONFIG_PAGE
-			|| wn->item->type == E_CONFIG_PAGE_START
-			|| wn->item->type == E_CONFIG_PAGE_FINISH))
-				break;
+		if (node->frame != page)
+			continue;
 
-		link = g_list_next (link);
-	}
+		if (node->item->type == E_CONFIG_PAGE)
+			break;
 
-	g_return_val_if_fail (wn != NULL, NULL);
+		if (node->item->type == E_CONFIG_PAGE_START)
+			break;
 
-	if (wn->frame != page)
-		wn = NULL;
+		if (node->item->type == E_CONFIG_PAGE_FINISH)
+			break;
+	}
+
+	/* FAIL: The widget is not in our list. */
+	if (link == NULL)
+		return NULL;
 
+	/* Find the corresponding GtkAssistant page index. */
 	if (page_index) {
-		if (wn) {
-			GtkAssistant *assistant = GTK_ASSISTANT (ec->widget);
-			gint index, count = gtk_assistant_get_n_pages (assistant);
+		GtkAssistant *assistant;
+		GtkWidget *nth_page;
+		gint ii, n_pages;
 
-			for (index = 0; index < count; index++) {
-				if (gtk_assistant_get_nth_page (assistant, index) == page)
-					break;
-			}
+		assistant = GTK_ASSISTANT (ec->widget);
+		n_pages = gtk_assistant_get_n_pages (assistant);
 
-			if (index == count)
-				index = -1;
-			*page_index = index;
-		} else {
-			*page_index = -1;
+		for (ii = 0; ii < n_pages; ii++) {
+			nth_page = gtk_assistant_get_nth_page (assistant, ii);
+			if (page == nth_page) {
+				*page_index = ii;
+				break;
+			}
 		}
+
+		g_warn_if_fail (ii < n_pages);
 	}
 
-	return wn;
+	return link;
 }
 
 static void
@@ -438,6 +446,7 @@ ec_assistant_check_current (EConfig *ec)
 	struct _finish_page_node *fp;
 	GtkAssistant *assistant;
 	GtkWidget *page;
+	GList *link;
 	gint page_no;
 
 	g_return_if_fail (GTK_IS_ASSISTANT (ec->widget));
@@ -452,8 +461,9 @@ ec_assistant_check_current (EConfig *ec)
 	page = gtk_assistant_get_nth_page (assistant, page_no);
 	g_return_if_fail (page != NULL);
 
-	wn = ec_assistant_find_page (ec, page, NULL);
-	g_return_if_fail (wn != NULL);
+	link = ec_assistant_find_page (ec, page, NULL);
+	g_return_if_fail (link != NULL);
+	wn = link->data;
 
 	/* this should come first, as the check function can change the finish state of the page */
 	gtk_assistant_set_page_complete (assistant, page, e_config_page_check (ec, wn->item->path));
@@ -477,38 +487,52 @@ ec_assistant_check_current (EConfig *ec)
 static gint
 ec_assistant_forward (gint current_page, gpointer user_data)
 {
+	GtkAssistant *assistant;
 	EConfig *ec = user_data;
-	struct _widget_node *wn;
-	gint next_page = current_page + 1;
+	struct _widget_node *node;
+	GtkWidget *page_widget;
 	GList *link = NULL;
+	gint next_page;
 
-	d(printf("next page from '%d'\n", current_page));
+	/* As far as we're concerned, the GtkAssistant is just an unordered
+	 * collection of pages.  Our sorted list of widget nodes determines
+	 * the next page. */
 
-	wn = ec_assistant_find_page (ec, gtk_assistant_get_nth_page (GTK_ASSISTANT (ec->widget), current_page), NULL);
+	assistant = GTK_ASSISTANT (ec->widget);
+	page_widget = gtk_assistant_get_nth_page (assistant, current_page);
+	link = ec_assistant_find_page (ec, page_widget, NULL);
 
-	/* XXX This is a little kludgy.  We have to find
-	 *     the GList node for the page we just found. */
-	if (wn != NULL)
-		link = g_list_find (ec->priv->widgets, wn);
+	g_return_val_if_fail (link != NULL, -1);
+	node = (struct _widget_node *) link->data;
 
-	if (link && link->next) {
-		for (link = link->next; link->next; link = link->next) {
-			wn = link->data;
+	/* If we're already on a FINISH page then we're done. */
+	if (node->item->type == E_CONFIG_PAGE_FINISH)
+		return -1;
 
-			if (!wn->empty && wn->frame != NULL
-			    && (wn->item->type == E_CONFIG_PAGE
-				|| wn->item->type == E_CONFIG_PAGE_START
-				|| wn->item->type == E_CONFIG_PAGE_FINISH))
-				break;
-		}
-	}
+	/* Find the next E_CONFIG_PAGE* type node. */
+	for (link = link->next; link != NULL; link = link->next) {
+		node = (struct _widget_node *) link->data;
+
+		if (node->empty || node->frame == NULL)
+			continue;
+
+		if (node->item->type == E_CONFIG_PAGE)
+			break;
 
-	if (link && link->next) {
-		wn = link->data;
-		ec_assistant_find_page (ec, wn->frame, &next_page);
-		d(printf(" is %s (%d)\n",wn->item->path, next_page));
+		if (node->item->type == E_CONFIG_PAGE_START)
+			break;
+
+		if (node->item->type == E_CONFIG_PAGE_FINISH)
+			break;
 	}
 
+	/* Find the corresponding GtkAssistant page number. */
+	if (link != NULL) {
+		node = (struct _widget_node *) link->data;
+		ec_assistant_find_page (ec, node->frame, &next_page);
+	} else
+		next_page = -1;
+
 	return next_page;
 }
 
@@ -519,7 +543,7 @@ ec_rebuild (EConfig *emp)
 	struct _widget_node *sectionnode = NULL, *pagenode = NULL;
 	GtkWidget *book = NULL, *page = NULL, *section = NULL, *root = NULL, *assistant = NULL;
 	gint pageno = 0, sectionno = 0, itemno = 0;
-	struct _widget_node *last_active_page = NULL;
+	GList *last_active_link = NULL;
 	gboolean is_assistant;
 	GList *link;
 
@@ -533,9 +557,20 @@ ec_rebuild (EConfig *emp)
 	/* because rebuild destroys pages, and destroying active page causes crashes */
 	is_assistant = emp->widget && GTK_IS_ASSISTANT (emp->widget);
 	if (is_assistant) {
+		GtkAssistant *assistant;
 		gint page_index = gtk_assistant_get_current_page (GTK_ASSISTANT (emp->widget));
-		if (page_index != -1)
-			last_active_page = ec_assistant_find_page (emp, gtk_assistant_get_nth_page (GTK_ASSISTANT (emp->widget), page_index), NULL);
+
+		assistant = GTK_ASSISTANT (emp->widget);
+		page_index = gtk_assistant_get_current_page (assistant);
+
+		if (page_index != -1) {
+			GtkWidget *nth_page;
+
+			nth_page = gtk_assistant_get_nth_page (
+				GTK_ASSISTANT (emp->widget), page_index);
+			last_active_link = ec_assistant_find_page (
+				emp, nth_page, NULL);
+		}
 		gtk_assistant_set_current_page (GTK_ASSISTANT (emp->widget), 0);
 	}
 
@@ -659,8 +694,7 @@ ec_rebuild (EConfig *emp)
 				break;
 			}
 
-			if (item->type == E_CONFIG_PAGE_FINISH || wn->widget == NULL) {
-				/* always rebuild finish page, to have it as the last page */
+			if (wn->widget == NULL) {
 				if (item->factory) {
 					page = item->factory(emp, item, root, wn->frame, wn->context->data);
 				} else {
@@ -920,11 +954,15 @@ ec_rebuild (EConfig *emp)
 		}
 	}
 
-	if (is_assistant && last_active_page) {
+	if (is_assistant && last_active_link != NULL) {
+		GtkAssistant *assistant;
+		struct _widget_node *wn;
 		gint page_index = -1;
 
-		ec_assistant_find_page (emp, last_active_page->frame, &page_index);
-		gtk_assistant_set_current_page (GTK_ASSISTANT (emp->widget), page_index);
+		wn = last_active_link->data;
+		assistant = GTK_ASSISTANT (emp->widget);
+		ec_assistant_find_page (emp, wn->frame, &page_index);
+		gtk_assistant_set_current_page (assistant, page_index);
 	}
 }
 
diff --git a/mail/em-account-editor.c b/mail/em-account-editor.c
index d0a7248..f854102 100644
--- a/mail/em-account-editor.c
+++ b/mail/em-account-editor.c
@@ -2044,14 +2044,14 @@ emae_setup_service (EMAccountEditor *emae, EMAccountEditorService *service, GtkB
 }
 
 static GtkWidget *
-emae_create_basic_assistant_page (EMAccountEditor *emae, GtkAssistant *assistant,
-				  const gchar *page_id, GtkWidget *old)
+emae_create_basic_assistant_page (EMAccountEditor *emae,
+                                  GtkAssistant *assistant,
+                                  const gchar *page_id)
 {
 	const gchar *title = NULL, *label = NULL;
 	GtkAssistantPageType page_type = GTK_ASSISTANT_PAGE_CONTENT;
 	GtkWidget *vbox, *lbl;
 	gboolean fill_space = FALSE;
-	gint index = -1;
 
 	g_return_val_if_fail (page_id != NULL, NULL);
 
@@ -2094,17 +2094,7 @@ emae_create_basic_assistant_page (EMAccountEditor *emae, GtkAssistant *assistant
 	if (g_ascii_strcasecmp (page_id, "start_page") == 0)
 		g_hash_table_insert (emae->priv->widgets, (gchar *)"start_page_label", lbl);
 
-	if (old) {
-		/* keep page on its previous index */
-		gint i, sz = gtk_assistant_get_n_pages (assistant);
-
-		for (i = 0; i < sz && index == -1; i++) {
-			if (gtk_assistant_get_nth_page (assistant, i) == old)
-				index = i;
-		}
-	}
-
-	gtk_assistant_insert_page (assistant, vbox, index);
+	gtk_assistant_append_page (assistant, vbox);
 	gtk_assistant_set_page_title (assistant, vbox, title);
 	gtk_assistant_set_page_type (assistant, vbox, page_type);
 
@@ -2186,7 +2176,7 @@ emae_identity_page (EConfig *ec, EConfigItem *item, GtkWidget *parent, GtkWidget
 	if (emae->type == EMAE_PAGES) {
 		gtk_box_pack_start ((GtkBox *)emae->pages[0], w, TRUE, TRUE, 0);
 	} else if (((EConfig *)priv->config)->type == E_CONFIG_ASSISTANT) {
-		GtkWidget *page = emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), "identity_page", old);
+		GtkWidget *page = emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), "identity_page");
 
 		gtk_box_pack_start (GTK_BOX (page), w, TRUE, TRUE, 0);
 
@@ -2223,7 +2213,7 @@ emae_receive_page (EConfig *ec, EConfigItem *item, GtkWidget *parent, GtkWidget
 	if (emae->type == EMAE_PAGES) {
 		gtk_box_pack_start ((GtkBox *)emae->pages[1], w, TRUE, TRUE, 0);
 	} else if (((EConfig *)priv->config)->type == E_CONFIG_ASSISTANT) {
-		GtkWidget *page = emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), "source_page", old);
+		GtkWidget *page = emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), "source_page");
 
 		gtk_box_pack_start (GTK_BOX (page), w, TRUE, TRUE, 0);
 
@@ -2700,7 +2690,7 @@ emae_send_page (EConfig *ec, EConfigItem *item, GtkWidget *parent, GtkWidget *ol
 	if (emae->type == EMAE_PAGES) {
 		gtk_box_pack_start ((GtkBox *)emae->pages[3], w, TRUE, TRUE, 0);
 	} else if (((EConfig *)priv->config)->type == E_CONFIG_ASSISTANT) {
-		GtkWidget *page = emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), "transport_page", old);
+		GtkWidget *page = emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), "transport_page");
 
 		gtk_box_pack_start (GTK_BOX (page), w, TRUE, TRUE, 0);
 
@@ -2907,7 +2897,7 @@ emae_management_page (EConfig *ec, EConfigItem *item, GtkWidget *parent, GtkWidg
 
 	w = priv->management_frame;
 	if (((EConfig *)priv->config)->type == E_CONFIG_ASSISTANT) {
-		GtkWidget *page = emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), "management_page", old);
+		GtkWidget *page = emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), "management_page");
 
 		gtk_widget_reparent (w, page);
 
@@ -2925,7 +2915,7 @@ emae_widget_assistant_page (EConfig *ec, EConfigItem *item, GtkWidget *parent, G
 	if (emae->type == EMAE_PAGES)
 		return NULL;
 
-	return emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), item->label, old);
+	return emae_create_basic_assistant_page (emae, GTK_ASSISTANT (parent), item->label);
 }
 
 /* plugin meta-data for "org.gnome.evolution.mail.config.accountAssistant" */