After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 333306 - Notice for duplicate sidebar pages
Notice for duplicate sidebar pages
Status: RESOLVED FIXED
Product: epiphany-extensions
Classification: Deprecated
Component: sidebar
master
Other Linux
: Normal normal
: ---
Assigned To: epiphany-extensions-maint
epiphany-extensions-maint
Depends on:
Blocks:
 
 
Reported: 2006-03-03 21:36 UTC by Wouter Bolsterlee (uws)
Modified: 2006-05-16 22:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed fix (1.50 KB, patch)
2006-03-03 21:38 UTC, Wouter Bolsterlee (uws)
none Details | Review
Fix typo (1.50 KB, patch)
2006-03-03 21:48 UTC, Wouter Bolsterlee (uws)
none Details | Review
Updated patch (1.83 KB, patch)
2006-05-16 22:03 UTC, Wouter Bolsterlee (uws)
accepted-commit_now Details | Review
tiny improvements (3.44 KB, patch)
2006-05-16 22:17 UTC, Wouter Bolsterlee (uws)
none Details | Review
screw bugzilla and cvs (1.91 KB, patch)
2006-05-16 22:21 UTC, Wouter Bolsterlee (uws)
none Details | Review

Description Wouter Bolsterlee (uws) 2006-03-03 21:36:28 UTC
Adding a sidebar page if it's already present is currently a no-op. Although there is no point adding a sidebar page twice, the user should be informed that the sidebar is already present. Adding a sidebar page for the first time also results in a confirmation dialog, so showing a notice makes the whole process consistent from a user perspective.

I'll attach a patch shortly.
Comment 1 Wouter Bolsterlee (uws) 2006-03-03 21:38:53 UTC
Created attachment 60604 [details] [review]
Proposed fix

The added code shows a notice to the user with instructions on how to use the sidebar.

This patch adds new strings (just like bug #333301), so it should not be committed before string freeze is over.
Comment 2 Wouter Bolsterlee (uws) 2006-03-03 21:40:59 UTC
Regarding this line in the original code (removed by my patch):

-	/* TODO Should we raise a dialog or perform an action here */

We should show a dialog. This also happens for new sidebar pages (confirmation). It's all about consistency.
Comment 3 Wouter Bolsterlee (uws) 2006-03-03 21:48:45 UTC
Created attachment 60605 [details] [review]
Fix typo

Fixed typo: "To use sidebar" -> "To use this sidebar"
Comment 4 Wouter Bolsterlee (uws) 2006-05-16 22:03:36 UTC
Created attachment 65617 [details] [review]
Updated patch

Don't use gtk_dialog_run. See #333301 for details.
Comment 5 Christian Persch 2006-05-16 22:09:56 UTC
Ok to commit with the nits from irc fixed.
Comment 6 Wouter Bolsterlee (uws) 2006-05-16 22:17:39 UTC
Created attachment 65619 [details] [review]
tiny improvements

- Corrected dialog primary text punctuation.
- Add translator comment
Comment 7 Wouter Bolsterlee (uws) 2006-05-16 22:19:56 UTC
Comment on attachment 65619 [details] [review]
tiny improvements

>Index: ephy-sidebar-extension.c
>===================================================================
>RCS file: /cvs/gnome/epiphany-extensions/extensions/sidebar/ephy-sidebar-extension.c,v
>retrieving revision 1.14
>diff -u -p -r1.14 ephy-sidebar-extension.c
>--- ephy-sidebar-extension.c	24 Apr 2006 10:27:01 -0000	1.14
>+++ ephy-sidebar-extension.c	16 May 2006 22:16:44 -0000
>@@ -386,7 +416,10 @@ ephy_sidebar_extension_add_sidebar_cb (E
> 	GtkWidget *dialog;
> 	struct ResponseCallbackData *cb_data;
> 	int i;
>-	
>+
>+	session = EPHY_SESSION (ephy_shell_get_session (ephy_shell));
>+	window = ephy_session_get_active_window (session);
>+
> 	/* See if the Sidebar is already added */
> 	for (i = 0 ; i < ephy_node_get_n_children (extension->priv->sidebars); i++)
> 	{
>@@ -395,13 +428,33 @@ ephy_sidebar_extension_add_sidebar_cb (E
> 		
> 		if (strcmp (node_url, url) == 0)
> 		{
>-			/* TODO Should we raise a dialog or perform an action here */
>+			dialog = gtk_message_dialog_new
>+				(GTK_WINDOW (window),
>+				 GTK_DIALOG_DESTROY_WITH_PARENT,
>+				 GTK_MESSAGE_INFO,
>+				 GTK_BUTTONS_OK,
>+				 /* Translators: %s is the sidebar title */
>+				 _("The sidebar already contains “%s”"), title);
>+
>+			gtk_message_dialog_format_secondary_text
>+				(GTK_MESSAGE_DIALOG (dialog),
>+				 _("To use this sidebar, select it from the list of available "
>+					 "sidebar pages."));
>+
>+			gtk_window_set_title (GTK_WINDOW (dialog), _("Add Sidebar"));
>+			gtk_window_set_icon_name (GTK_WINDOW (dialog), "web-browser");
>+
>+			gtk_window_group_add_window (ephy_gui_ensure_window_group
>+					(GTK_WINDOW (window)), GTK_WINDOW (dialog));
>+
>+			g_signal_connect (dialog, "response", G_CALLBACK
>+					(gtk_widget_destroy), NULL);
>+
>+			gtk_widget_show (GTK_WIDGET (dialog));
>+
> 			return TRUE;
> 		}
> 	}
>-
>-	session = EPHY_SESSION (ephy_shell_get_session (ephy_shell));
>-	window = ephy_session_get_active_window (session);
> 
> 	dialog = gtk_message_dialog_new
> 		(GTK_WINDOW (window),
Comment 8 Wouter Bolsterlee (uws) 2006-05-16 22:21:58 UTC
Created attachment 65620 [details] [review]
screw bugzilla and cvs
Comment 9 Wouter Bolsterlee (uws) 2006-05-16 22:26:00 UTC
2006-05-17  Wouter Bolsterlee  <uws+gnome@xs4all.nl>

        * extensions/sidebar/ephy-sidebar-extension.c:
        (ephy_sidebar_extension_add_sidebar_cb):

        Show a notification dialog when a duplicate sidebar is
        added. Fixes bug #333306.