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 508555 - enable browser for all sources by default
enable browser for all sources by default
Status: RESOLVED OBSOLETE
Product: rhythmbox
Classification: Other
Component: User Interface
0.11.x
Other Linux
: Normal enhancement
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-01-10 16:50 UTC by Mirco Müller
Modified: 2018-05-24 13:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to implement browser by default (528 bytes, patch)
2008-01-10 16:51 UTC, Mirco Müller
none Details | Review
hopefully corrected version of the previous patch (985 bytes, patch)
2008-01-11 03:29 UTC, Mirco Müller
none Details | Review
change default browser visibility for sources that don't store it in gconf (400 bytes, patch)
2008-01-12 12:03 UTC, Jonathan Matthew
rejected Details | Review
proper browser-state handling for ipod/mtp-devices (via gconf) (5.53 KB, patch)
2008-01-22 16:24 UTC, Mirco Müller
committed Details | Review

Description Mirco Müller 2008-01-10 16:50:45 UTC
The summary says it all. The applied 1-line-patch does improve the default behaviour of rb by showing the user the browser right away for all sources. A user will be able to search the music-collection of a selected source by simple pointing- and clicking-actions without the need to enter search key-words via the keyboard. Not everybody is a geek and can touch-type :)
Comment 1 Mirco Müller 2008-01-10 16:51:34 UTC
Created attachment 102526 [details] [review]
patch to implement browser by default
Comment 2 Mirco Müller 2008-01-10 23:29:35 UTC
Argl, the patch does apparently not work for someone else who tested it. I'm looking into it.
Comment 3 Mirco Müller 2008-01-11 03:28:03 UTC
Comment on attachment 102526 [details] [review]
patch to implement browser by default

--- rhythmbox/plugins/mtpdevice/rb-mtp-plugin.c	2007-12-26 13:00:08.000000000 +0100
+++ rhythmbox-new/plugins/mtpdevice/rb-mtp-plugin.c	2008-01-11 04:02:30.000000000 +0100
@@ -241,6 +241,7 @@
 	RBSource *source;
 
 	source = RB_SOURCE (rb_mtp_source_new (plugin->shell, device, udi));
+	rb_source_browser_toggled (source, TRUE);
 
 	rb_shell_append_source (plugin->shell, source, NULL);
 	plugin->mtp_sources = g_list_prepend (plugin->mtp_sources, source);
--- rhythmbox/plugins/ipod/rb-ipod-plugin.c	2008-01-11 03:58:27.000000000 +0100
+++ rhythmbox-new/plugins/ipod/rb-ipod-plugin.c	2008-01-11 04:01:02.000000000 +0100
@@ -203,7 +203,7 @@
 	if (rb_ipod_is_volume_ipod (volume)) {
 		RBSource *src;
 		src = RB_SOURCE (rb_ipod_source_new (plugin->shell, volume));
-
+		rb_source_browser_toggled (src, TRUE);
 		plugin->ipod_sources = g_list_prepend (plugin->ipod_sources, src);
 		g_signal_connect_object (G_OBJECT (src),
 					 "deleted", G_CALLBACK (rb_ipod_plugin_source_deleted),
Comment 4 Mirco Müller 2008-01-11 03:29:30 UTC
Created attachment 102566 [details] [review]
hopefully corrected version of the previous patch
Comment 5 Jonathan Matthew 2008-01-12 12:03:13 UTC
Created attachment 102656 [details] [review]
change default browser visibility for sources that don't store it in gconf

I think it would be better to improve browser state tracking.  If we committed a patch like this, I'd probably find it annoying to have the browser visible by default on all my playlists.

For the device-based sources, we should just add gconf keys - /apps/rhythmbox/plugins/ipod/browser_visible for ipods etc.  For the minority who have multiple devices of one kind, are they really going to want the browser shown for one device but not another?

We'd need to do something smarter for playlists, though.  Storing state in gconf doesn't seem right to me, though.  Should it be stored in the playlists file?
Comment 6 Christophe Fergeau 2008-01-12 12:31:14 UTC
For ipods, what about having /apps/rhythmbox/plugins/ipod/XXXXXXX-state/browser_visible keys, with XXXXXXX being the device serial number ? Actually, something that might be better, but is ipod specific, is to store the "browser visible" preference on the ipod, I'm pretty sure there's a field for that in the ipod database (itunes uses it)
Comment 7 Mirco Müller 2008-01-22 16:24:40 UTC
Created attachment 103456 [details] [review]
proper browser-state handling for ipod/mtp-devices (via gconf)
Comment 8 Jonathan Matthew 2008-01-31 12:25:05 UTC
I've been trying my patch for a while, and it's really quite annoying.

The patch for ipod/mtp sources looks OK, except perhaps the gconf keys should be under plugins/ipod/ and plugins/mtp/.  I haven't tested it, as I don't have an ipod or an mtp device that works.
Comment 9 Mirco Müller 2008-01-31 12:59:22 UTC
Other plugins save their UI-state under /apps/rhythmbox/state/<plugin> too (e.g. daap, iradio, library, podcast). Thus I'm only sticking to encountered conventions instead of introducing new ones and scattering stuff all over the place.
Comment 10 Jonathan Matthew 2008-01-31 23:28:13 UTC
That's true, but it's that way because we started using those keys before daap and iradio became plugins.
 
I think we should try to keep plugins contained so that conflicts are impossible, but it's probably not worth the effort of moving already established gconf keys.
Comment 11 Jonathan Matthew 2008-04-14 09:45:38 UTC
Committed.  Not sure why I left this lying around for so long..
Comment 12 GNOME Infrastructure Team 2018-05-24 13:05:20 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/rhythmbox/issues/491.