I've found the source of this problem. I was going to path it, but then
I had a closer look and decided not to. The problem is in
debian/patches/19_all_play_audiocd, and it is easily patched with the
information I will provide in this mail. However, you shouldn't do it.
The proper solution is to delete debian/patches/19_all_play_audiocd.
19_all_play_audiocd is a complete WTF, and should never have been
applied. I'll give you the short summary first, and the long and boring
one afterwards:
* The UI is completely wrong
* The code is buggy (obviously)
* It provides basically no new functionality and only creates extra
work for the maintainer
Long description: The patch includes the tab CD Audio in Preferences.
This looks like a pretty wierd place to put it if you ask me, I would
expect this setting to be in the settings of the input plugin. And wait
a second, it is! Actually, for the Play - AudioCD menu item to work,
both the directories must be set to the same thing! This is of course
completely horrible from a usability point of view.
The Play - CD Audio menu item itself is also from the patch, and is
in fact the only real functionality it provides. This menu button does
the same thing as pressing Play - Play Directory and selecting the
directory would do. This only serves to make people think there's
something different with audio CDs. There isn't, they work just as
normal directories. The menu item is only creating confusion.
Okay, over to the code. In xmms/prefswin.c there is a function
prefswin_audiocd_browse_cb (added by 19_all_play_audiocd). I'll quote it
here with numbered lines:
1 static gint prefswin_audiocd_browse_cb(GtkWidget * w, gpointer data)
2 {
3 GtkWidget *prefswin_audiocd_browser;
4 prefswin_audiocd_browser = xmms_create_dir_browser(_(Select directory
to add:), gtk_entry_get_text(GTK_ENTRY(prefswin_audiocd_cddadirectory)),
GTK_SELECTION_SINGLE, prefswin_audiocd_browse_handler);
5 gtk_signal_connect(GTK_OBJECT(prefswin_audiocd_browser), destroy,
GTK_SIGNAL_FUNC(gtk_widget_destroyed), prefswin_audiocd_browser);
6 gtk_window_set_transient_for(GTK_WINDOW(prefswin_audiocd_browser),
GTK_WINDOW(prefswin));
7 gtk_widget_show(prefswin_audiocd_browser);
8 return (TRUE);
9 }
Notice that *prefswin_audiocd_browser declared in line 3 is local to the
function. When I removed the gtk_signal_connect in line 5, everything
seems to work fine. I found no good docs for GTK 1.x on this, but it's
probably about the same in 2.x I hope. According to
http://developer.gimp.org/api/2.0/gtk/GtkWidget.html#gtk-widget-destroyed
gtk_widget_destroyed sets *widget_pointer to NULL if widget_pointer !=
NULL. [...] Useful for example to avoid multiple copies of the same
dialog.
Looking around the source, it seems gtk_widget_destroyed is used mostly
on global variables, which makes sense for detecting if the window is
already open and just stashed away somewhere. Currently you can open an
infinite amount of identical windows from the Browse button. The
solution to this is declaring *prefswin_audiocd_browser outside the
function, and wrapping the entire function body in an
if (!prefswin_audiocd_browser) { /* body */ }.
This is how it is done in all other windows in XMMS using
xmms_create_dir_browser.
As a sidenote, returning int from this function is also wierd. All other
*_cb functions return void, but this one returns TRUE unconditionally. I
don't know why, but suspect incompetence.
The maintainer hell part is pretty obvious. Today there are 29 patches
in debian/patches, quite a few of then weird stuff nobody asked for.
Take for instance 12_all_sortbytime, which sorts the playlist after the
length of the tracks. Who on earth would want that? Next time the package is
updated from upstream, most of these patches will fail. The poor
maintainer will have to apply almost every patch manually. My guess is
that it will not be very fun. The effect in the long run will probably
be that it will not be very tempting to update the source from upstream,
in practise creating a fork.
That being said, removing the patch now will probably force you to
re-apply patch 20 to 51 manually. It might be a good idea to just update
the entire thing from CVS first, since that will be about the same
amount of work and will probably fix a few other bugs as well. For
instance I hear rumors on IRC about an accidentally shuffled enum in the
CVS snapshot used in the current package. It will also add functionality
I'm waiting for ;o)
--
Knut Auvor Grythe
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
