Bug 628141 - Duplicate signal connections in EMailReader

EMailShellContent implements the EMailReader interface but acts as a
proxy for EMailPanedView, from which it obtains MessageList and EWebView
widgets.  The problem was both classes call e_mail_reader_init_private()
which connects to signals emitted from the MessageList and EWebView
widgets.  But since EMailShellContent is a proxy for EMailPanedView,
the signals were being connected twice.

This commit does away with e_mail_reader_init_private(), instead adding
options to e_mail_reader_init() to control what parts of initialization
to run.  It's an ugly and temporary hack.

I'm beginning to realize EMailReader is too bloated and needs rethought.
EMailReader should just manage actions.  EMailView should own and manage
the widgets, and EMailReader should just have a get_mail_view() method
so it has access to those widgets.  That way the EMailView subclasses
won't have to implement EMailReader themselves and wind up allocating
a bunch of duplicate, unused actions.

It's too close to a stable release to rip these interfaces apart and
reorganize them.  I'll try to do that for 2.33 to help make the design
more intuitive.

5 files changed, 25 insertions, 32 deletions

diff --git a/mail/e-mail-browser.c b/mail/e-mail-browser.c
index 2ace9e4dd6..34b4313a6f 100644
--- a/mail/e-mail-browser.c
+++ b/mail/e-mail-browser.c
@@ -543,7 +543,7 @@ mail_browser_constructed (GObject *object)
 		web_view, "status-message",
 		G_CALLBACK (mail_browser_status_message_cb), object);
 
-	e_mail_reader_init (reader);
+	e_mail_reader_init (reader, TRUE, TRUE);
 
 	action_group = priv->action_group;
 	gtk_action_group_set_translation_domain (action_group, domain);
diff --git a/mail/e-mail-paned-view.c b/mail/e-mail-paned-view.c
index e81a7bbbef..ccbd6a2077 100644
--- a/mail/e-mail-paned-view.c
+++ b/mail/e-mail-paned-view.c
@@ -611,7 +611,7 @@ mail_paned_view_constructed (GObject *object)
 
 	/* Message list customizations. */
 
-	e_mail_reader_init_private (E_MAIL_READER (object));
+	e_mail_reader_init (E_MAIL_READER (object), FALSE, TRUE);
 
 	reader = E_MAIL_READER (object);
 	message_list = e_mail_reader_get_message_list (reader);
diff --git a/mail/e-mail-reader.c b/mail/e-mail-reader.c
index 805ed615d8..d4689c375f 100644
--- a/mail/e-mail-reader.c
+++ b/mail/e-mail-reader.c
@@ -2996,7 +2996,9 @@ e_mail_reader_default_init (EMailReaderInterface *interface)
 }
 
 void
-e_mail_reader_init (EMailReader *reader)
+e_mail_reader_init (EMailReader *reader,
+                    gboolean init_actions,
+                    gboolean connect_signals)
 {
 	EShell *shell;
 	EShellBackend *shell_backend;
@@ -3008,6 +3010,7 @@ e_mail_reader_init (EMailReader *reader)
 	GtkWidget *message_list;
 	GConfBridge *bridge;
 	GtkAction *action;
+	gboolean sensitive;
 	const gchar *action_name;
 	const gchar *key;
 
@@ -3023,6 +3026,9 @@ e_mail_reader_init (EMailReader *reader)
 
 	web_view = em_format_html_get_web_view (formatter);
 
+	if (!init_actions)
+		goto connect_signals;
+
 	/* The "mail-forward" action is special: it uses a GtkMenuToolButton
 	 * for its toolbar item type.  So we have to create it separately. */
 
@@ -3046,7 +3052,8 @@ e_mail_reader_init (EMailReader *reader)
 	   (if possible and if that configuration option is enabled), or else
 	   it will reply to all. The word "Group" was chosen because it covers
 	   either of those, without too strongly implying one or the other. */
-	menu_tool_action = e_menu_tool_action_new ("mail-reply-group", _("Group Reply"),
+	menu_tool_action = e_menu_tool_action_new (
+		"mail-reply-group", _("Group Reply"),
 		_("Reply to the mailing list, or to all recipients"), NULL);
 
 	gtk_action_set_icon_name (
@@ -3104,8 +3111,9 @@ e_mail_reader_init (EMailReader *reader)
 
 	action_name = "mail-goto-folder";
 	action = e_mail_reader_get_action (reader, action_name);
+	sensitive = e_mail_reader_get_enable_show_folder (reader);
 	g_object_set (action, "visible", FALSE, NULL);
-	gtk_action_set_sensitive (action, e_mail_reader_get_enable_show_folder (reader));
+	gtk_action_set_sensitive (action, sensitive);
 
 	action_name = "mail-goto-nexttab";
 	action = e_mail_reader_get_action (reader, action_name);
@@ -3167,31 +3175,10 @@ e_mail_reader_init (EMailReader *reader)
 		action, "active",
 		web_view, "caret-mode");
 
-	e_mail_reader_init_private (reader);
-}
+connect_signals:
 
-void
-e_mail_reader_init_private (EMailReader *reader)
-{
-	EMFormatHTML *formatter;
-	EWebView *web_view;
-	GtkWidget *message_list;
-
-	g_return_if_fail (E_IS_MAIL_READER (reader));
-
-	formatter = e_mail_reader_get_formatter (reader);
-	message_list = e_mail_reader_get_message_list (reader);
-
-	web_view = em_format_html_get_web_view (formatter);
-
-	/* Disconnect signals, if any, to not be connected twice */
-	g_signal_handlers_disconnect_by_func (web_view, mail_reader_key_press_event_cb, reader);
-	g_signal_handlers_disconnect_by_func (message_list, mail_reader_message_selected_cb, reader);
-	g_signal_handlers_disconnect_by_func (message_list, mail_reader_emit_folder_loaded, reader);
-	g_signal_handlers_disconnect_by_func (message_list, mail_reader_double_click_cb, reader);
-	g_signal_handlers_disconnect_by_func (message_list, mail_reader_key_press_cb, reader);
-	g_signal_handlers_disconnect_by_func (message_list, e_mail_reader_changed, reader);
-	g_signal_handlers_disconnect_by_func (reader, mail_reader_destroy, NULL);
+	if (!connect_signals)
+		goto init_private;
 
 	/* Connect signals. */
 	g_signal_connect_swapped (
@@ -3218,10 +3205,15 @@ e_mail_reader_init_private (EMailReader *reader)
 		message_list, "selection-change",
 		G_CALLBACK (e_mail_reader_changed), reader);
 
+init_private:
+
+	/* Initialize a private struct. */
+
 	g_object_set_qdata_full (
 		G_OBJECT (reader), quark_private,
 		g_slice_new0 (EMailReaderPrivate),
 		(GDestroyNotify) mail_reader_private_free);
+
 	g_signal_connect (
 		reader, "destroy",
 		G_CALLBACK (mail_reader_destroy), NULL);
diff --git a/mail/e-mail-reader.h b/mail/e-mail-reader.h
index b2247048be..dbd47e10e7 100644
--- a/mail/e-mail-reader.h
+++ b/mail/e-mail-reader.h
@@ -109,8 +109,9 @@ struct _EMailReaderInterface {
 };
 
 GType		e_mail_reader_get_type		(void);
-void		e_mail_reader_init		(EMailReader *reader);
-void		e_mail_reader_init_private (EMailReader *reader);
+void		e_mail_reader_init		(EMailReader *reader,
+						 gboolean init_actions,
+						 gboolean connect_signals);
 void		e_mail_reader_changed		(EMailReader *reader);
 guint32		e_mail_reader_check_state	(EMailReader *reader);
 void		e_mail_reader_update_actions	(EMailReader *reader,
diff --git a/modules/mail/e-mail-shell-view-private.c b/modules/mail/e-mail-shell-view-private.c
index a794329501..907eb18259 100644
--- a/modules/mail/e-mail-shell-view-private.c
+++ b/modules/mail/e-mail-shell-view-private.c
@@ -665,7 +665,7 @@ e_mail_shell_view_private_constructed (EMailShellView *mail_shell_view)
 			G_CALLBACK (mail_shell_view_prepare_for_quit_cb),
 			mail_shell_view, G_CONNECT_SWAPPED);
 
-	e_mail_reader_init (reader);
+	e_mail_reader_init (reader, TRUE, FALSE);
 	e_mail_shell_view_actions_init (mail_shell_view);
 	e_mail_shell_view_update_search_filter (mail_shell_view);