I'm going to take this, but I can't see a clean way to do it without re-doing the interface. All I've thought of so far is to get all the entries in Totem's GtkRecentManager when the open location dialog is shown, and add them to the GnomeEntry's history list, but not save them to the history file. Ideally, the GnomeEntry should be replaced with some form of GtkRecentChooser* control (GnomeEntry is deprecated, isn't it?), but I can't find one which will also allow you to type in a location. :-\

You probably need a GtkEntry, with a GtkEntryCompletion object that would use the data from the recent files, or better, from data stored in the recent files but wouldn't be shown in the File menu.

That would allow for a "Streams" sidebar to be added, where we'd get the same data as in the drop-down menu.

Created attachment 79282 [details] [review]
Patch using current GnomeEntry

Before I have a crack at doing it with a GtkEntry, here's my patch to get it going using the existing GnomeEntry, for reference purposes.

Created attachment 79288 [details] [review]
Patch changing it to use GtkEntry

Here's a patch using GtkEntry, and GtkEntryCompletion feeding off Totem's GtkRecentManager as before.

Hmmm...should I have kept that HAVE_GTK_ONLY #ifdef, or should it be removed now that the code doesn't use GnomeEntry and its associated history code?

Created attachment 79524 [details] [review]
Updated to remove unnecessary #ifndef

Updated the patch to remove an unnecessary #ifndef and make sure recent changes to totem-menu.c don't upset it.

Comment on attachment 79524 [details] [review]
Updated to remove unnecessary #ifndef

>Index: src/totem-menu.c
>===================================================================
>--- src/totem-menu.c	(revision 3827)
>+++ src/totem-menu.c	(working copy)
>@@ -451,30 +451,6 @@
> 	}
> }
> 
>-static gint
>-totem_compare_recent_items (GtkRecentInfo *a, GtkRecentInfo *b)
>-{
>-	gboolean has_totem_a, has_totem_b;
>-
>-	has_totem_a = gtk_recent_info_has_group (a, "Totem");
>-	has_totem_b = gtk_recent_info_has_group (b, "Totem");
>-
>-	if (has_totem_a && has_totem_b) {
>-		time_t time_a, time_b;
>-
>-		time_a = gtk_recent_info_get_modified (a);
>-		time_b = gtk_recent_info_get_modified (b);
>-
>-		return (time_b - time_a);
>-	} else if (has_totem_a) {
>-		return -1;
>-	} else if (has_totem_b) {
>-		return 1;
>-	}
>-
>-	return 0;
>-}
>-
> static void
> totem_recent_manager_changed_callback (GtkRecentManager *recent_manager, Totem *totem)
> {
>Index: src/totem.c
>===================================================================
>--- src/totem.c	(revision 3827)
>+++ src/totem.c	(working copy)
>@@ -682,13 +682,37 @@
> 	totem_action_open_dialog (totem, NULL, TRUE);
> }
> 
>+gint
>+totem_compare_recent_items (GtkRecentInfo *a, GtkRecentInfo *b)
>+{
>+	gboolean has_totem_a, has_totem_b;
>+
>+	has_totem_a = gtk_recent_info_has_group (a, "Totem");
>+	has_totem_b = gtk_recent_info_has_group (b, "Totem");

The "recent streams" in the open url dialogue shouldn't be kept in the same group as the local files that will be added. Use "TotemStreams" instead, for example.

>+	if (has_totem_a && has_totem_b) {
>+		time_t time_a, time_b;
>+
>+		time_a = gtk_recent_info_get_modified (a);
>+		time_b = gtk_recent_info_get_modified (b);
>+
>+		return (time_b - time_a);
>+	} else if (has_totem_a) {
>+		return -1;
>+	} else if (has_totem_b) {
>+		return 1;
>+	}
>+
>+	return 0;
>+}
>+
> //FIXME move the URI to a separate widget
> void
> totem_action_open_location (Totem *totem)
> {
> 	GladeXML *glade;
> 	char *mrl;
>-	GtkWidget *dialog, *entry, *gentry;
>+	GtkWidget *dialog, *entry;
> 	int response;
> 	const char *filenames[2];
> 
>@@ -699,8 +723,40 @@
> 
> 	dialog = glade_xml_get_widget (glade, "open_uri_dialog");
> 	entry = glade_xml_get_widget (glade, "uri");
>-	gentry = glade_xml_get_widget (glade, "uri_list");
> 
>+	GtkEntryCompletion *completion = gtk_entry_completion_new();
>+	GtkTreeModel *model = GTK_TREE_MODEL (gtk_list_store_new (1, G_TYPE_STRING));
>+
>+	gtk_entry_set_completion (GTK_ENTRY (entry), completion);
>+	gtk_entry_completion_set_model (completion, model);
>+	gtk_entry_completion_set_text_column (completion, 0);
>+
>+	// Add items in Totem's GtkRecentManager to the uri_list GtkEntry's GtkEntryCompletion

Please no C++ style comments.

>+	GList *recent_items = gtk_recent_manager_get_items (totem->recent_manager);
>+
>+	if ( recent_items != NULL )

Please follow the coding style. Should be:
if (recent_items != NULL)

>+	{
>+		recent_items = g_list_sort (recent_items, (GCompareFunc) totem_compare_recent_items);
>+
>+		GList *p;
>+		GtkRecentInfo *recent_info;
>+		GtkTreeIter iter;
>+
>+		for (p = recent_items; p != NULL; p = p->next)
>+		{
>+			recent_info = p->data;
>+
>+			if (!gtk_recent_info_has_group (recent_info, "Totem"))

"TotemStreams" again.

>+				continue;
>+
>+			gtk_list_store_append (GTK_LIST_STORE (model), &iter);
>+			gtk_list_store_set (GTK_LIST_STORE (model), &iter, 0, gtk_recent_info_get_uri (recent_info), -1);
>+			gtk_recent_info_unref (recent_info);
>+		}
>+	}
>+
>+	g_list_free (recent_items);
>+
> 	response = gtk_dialog_run (GTK_DIALOG (dialog));
> 
> 	if (response == GTK_RESPONSE_OK)
>@@ -717,10 +773,6 @@
> 			mrl = totem_playlist_get_current_mrl (totem->playlist);
> 			totem_action_set_mrl_and_play (totem, mrl);
> 			g_free (mrl);
>-#ifndef HAVE_GTK_ONLY
>-			gnome_entry_append_history (GNOME_ENTRY (gentry),
>-					TRUE, uri);
>-#endif /* !HAVE_GTK_ONLY */
> 		}
> 	}
> 
>Index: src/totem.h
>===================================================================
>--- src/totem.h	(revision 3827)
>+++ src/totem.h	(working copy)
>@@ -28,6 +28,7 @@
> #ifndef __TOTEM_H__
> #define __TOTEM_H__
> 
>+#include <gtk/gtk.h>
> #include <gconf/gconf-client.h>
> #include <gtk/gtkwidget.h>
> #include "bacon-video-widget.h"
>@@ -71,4 +72,7 @@
> void    totem_action_play_media_device		(Totem *totem,
> 						 const char *device);
> 
>+gint	totem_compare_recent_items		(GtkRecentInfo *a,
>+						 GtkRecentInfo *b);
>+
> #endif /* __TOTEM_H__ */

<snip>

Created attachment 79535 [details] [review]
Fixed issues mentioned above

Fixed those issues, and also made listing of recent items in the location window more efficient.

To recap, the completion for the Open Location window feeds off Totem's RecentManager items marked with "TotemStreams", and URIs opened via this window are added to the RecentManager as "TotemStreams" and "Totem". The recent list in the menu feeds off all "Totem" RecentManager items (including those opened from the Open Location window).

Should I implement lazy loading (http://log.emmanuelebassi.net/documentation/lazy-loading/) for the completion items in the Open Location window?

Given how often one would use the Open URL, I don't think it's needed.

2007-01-08  Bastien Nocera  <hadess@hadess.net>

        * data/uri.glade:
        * src/totem.c: (totem_compare_recent_stream_items),
        (totem_action_add_recent_stream), (totem_action_open_location):
        Patch from Philip Withnall <bugzilla@tecnocode.co.uk> to remember the
        URLs added in the "Open URL", and add them to the Recent Files
        (Closes: #338020)

Also see bug 394096, about doing some further cleanups.