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 308388 - for DVD/audio CD, should not offer to "open" but to "play"
for DVD/audio CD, should not offer to "open" but to "play"
Status: RESOLVED FIXED
Product: gnome-applets
Classification: Other
Component: Disk Mounter (drivemount)
2.10.x
Other All
: Normal enhancement
: ---
Assigned To: gnome-applets Maintainers
gnome-applets Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-06-20 13:07 UTC by Emmanuel Touzery
Modified: 2005-07-13 14:09 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
very rough patch (4.15 KB, patch)
2005-06-26 19:10 UTC, Emmanuel Touzery
none Details | Review
remove gross error. still useless. (4.27 KB, patch)
2005-06-26 19:14 UTC, Emmanuel Touzery
needs-work Details | Review
actually works (6.06 KB, patch)
2005-06-27 04:02 UTC, Emmanuel Touzery
committed Details | Review
patch with nice detection for CDA/DVDv (7.71 KB, patch)
2005-07-01 16:47 UTC, Emmanuel Touzery
none Details | Review
i forgot a g_free() in the previous patch... (7.74 KB, patch)
2005-07-01 17:41 UTC, Emmanuel Touzery
none Details | Review
patch updated to CVS (2.96 KB, patch)
2005-07-02 06:06 UTC, Emmanuel Touzery
committed Details | Review

Description Emmanuel Touzery 2005-06-20 13:07:13 UTC
For a maximum ease of use and simplicity, (in my opinion), for DVDs and audio
CDs the drive mount applet should offer "play media" instead of "open".

The reasonning is that almost nobody wants to see the VIDEO_TS and AUDIO_TS on a
DVD, same for an audio CD. The application to launch should be the same than the
one launched automatically on inserting a CD/DVD (dialog "drives and media
preferences").

A user who would not know which application plays DVDs/CDs would have to eject
the media and reinsert it, counting on the "autoplay" feature of GNOME that will
launch the appropriate players. That would be the case for the common case of
booting with the media already in the drive; then the autoplay wouldn't be
triggered.

I'll submit the same bug for the right-click menu on the media icon on the
desktop, too.
Comment 1 Emmanuel Touzery 2005-06-20 13:20:40 UTC
the bug for the desktop icon is bug #308389
Comment 2 Emmanuel Touzery 2005-06-26 19:09:44 UTC
i started writing a patch, however I can't seem to be able to run my version of
the applet, either running "make install" (with the right prefix, which
overwrites /usr/lib/gnome-applets/drivemount_applet2 ) or copying the binary by
hand. is there any document for GNOME2 explaining how to replace the original
applet by my modified one?
I'll send the patch in the next attachment for information, but it just
compiles, it's completely untested. i also didn't find how to run the action for
CDA/DVD easily so for now there's a stupid copy-paste from GVM which can at
least be simplified if there's no simpler way to do that.

any advice appreciated!
Comment 3 Emmanuel Touzery 2005-06-26 19:10:34 UTC
Created attachment 48371 [details] [review]
very rough patch

this patch is not tested, and definitely not final! it's just indicating how
far i am.
Comment 4 Emmanuel Touzery 2005-06-26 19:14:17 UTC
Created attachment 48372 [details] [review]
remove gross error. still useless.

remove a stupid error in the previous patch. still useless.
Comment 5 Danielle Madeley 2005-06-26 23:35:28 UTC
Here is some information that might be useful to running and debugging your applet:
    http://www.davyd.id.au/articles/debugging-gnome-applets.shtml

Thanks for the great work so far.
Comment 6 Emmanuel Touzery 2005-06-27 04:01:10 UTC
I made a new version of the patch. now it actually works, and it's only got two
issues (that i'm aware of ;-) ):

1. copy-pasted from code from gnome-volume-manager for running the autoplay
action for audioCD+DVD video

2. on my computer (ubuntu hoary) i'm not getting from gnome-vfs
GNOME_VFS_DEVICE_TYPE_VIDEO_DVD GNOME_VFS_DEVICE_TYPE_AUDIO_CD for DVDv and CDA,
but simply GNOME_VFS_DEVICE_TYPE_CDROM. the funny thing is that for DVDs the
icon is set to a DVD-specific icon and for CDA the description is set to "Audio
CD". So, while it's ugly, instead of using the device_type which is simply CD,
I'm using the description and icon name...

if there's no better way, 1. might be OK. I guess 2. is unnaceptable, but I
don't know what to do about it? Maybe it's a bug that is fixed in 2.11.x, maybe
I'm badly using gnome-vfs?

patch follows.
Comment 7 Emmanuel Touzery 2005-06-27 04:02:28 UTC
Created attachment 48394 [details] [review]
actually works

this patch works for me, code has some issues, see my bugzilla message +
comments in the code.
Comment 8 Emmanuel Touzery 2005-06-27 04:04:01 UTC
btw, Davyd Madeley, thank you very much for your link, wouldn't get anywhere
without it. it helped a lot. maybe it should be added to developer.gnome.org?
that's where i was unsuccessfully looking for it..
Comment 9 Emmanuel Touzery 2005-06-27 18:11:24 UTC
just a question.. I started writing the detection whether it's a DVD video or an
audio CD in the drive properly (fixing point 2 of the previous patch). I'll do
exactly like gnome-volume-manager (checking the subdir VIDEO_TS for DVD, using
HAL for audioCD).
I already did it for DVD video, the problem is using HAL for audioCD. I don't
know autoconf/automake, can someone tell me how to make the applet link with HAL
so I can call its functions? (I think i have to change Makefile.am and maybe
configure.in files).
I hope that HAL dependency is OK.

after that's done, I think the patch will be fine for inclusion if you want it.
Comment 10 Danielle Madeley 2005-06-28 10:39:47 UTC
Here is an example of the easy way to do it.
http://cvs.gnome.org/viewcvs/gnome-netstatus/configure.in?r1=1.112&r2=1.112.2.1

It would be much better however, to have an optional HAL dependancy that is
exposed through a HAVE_HAL define. This can then also be used in battstat.

Take a look at some of the existing checks, such as the one for libnotify. If
you're still stuck, I'll help you out.
Comment 11 Emmanuel Touzery 2005-06-28 12:26:14 UTC
where is there a check for libnotify? i don't see the check in
http://cvs.gnome.org/viewcvs/gnome-netstatus/configure.in?rev=1.112.2.1&view=markup

thank you very much for the help!
Comment 12 Danielle Madeley 2005-06-28 12:37:51 UTC
Oh, sorry. Look in the gnome-applets' configure.in

http://cvs.gnome.org/viewcvs/gnome-applets/configure.in?r1=1.370&r2=1.371
Comment 13 Danielle Madeley 2005-07-01 12:14:19 UTC
Ok, I've committed the current version, as it seems to work ok. I want to get
the features out there.
Comment 14 Emmanuel Touzery 2005-07-01 12:32:54 UTC
OK, i'll send a new version as soon as ready, because besides ugliness of
CDA/DVDv detection, I believe that gnome_vfs_volume_get_display_name() returns a
translated string, so CDA detection will only work in a C locale.

I'll send the new version ASAP.
Comment 15 Emmanuel Touzery 2005-07-01 16:47:03 UTC
Created attachment 48513 [details] [review]
patch with nice detection for CDA/DVDv

this patch fixes the detection of CDA and DVDv. For DVDv it does like
gnome-volume-manager, check for a subdirectory VIDEO_TS. For audioCD, I first
wanted to do like gvm using HAL but the HAL on my computer is 0.4.7 using the
obsolete API and since for now I rather not install the HAL-0.5, I went another
way: I check the gnome_vfs activation_uri of the device. for audioCD is
"cdda://<device>". for normal CDs it is "file://<mount_point>". That's the way
I detect if it's an audioCD. it could break if that activation uri system would
be changed, i guess HAL would be more reliable, but it's definitely better than
the previous patch.
I also replaced the icon for the play action from GTK_STOCK_CDROM to
GTK_STOCK_MEDIA_PLAY which is probably more appropriate.
Comment 16 Emmanuel Touzery 2005-07-01 17:41:00 UTC
Created attachment 48517 [details] [review]
i forgot a g_free() in the previous patch...

i forgot a g_free() in check_dvd_video(). that patch fixes it.

+	g_free (mount_path);
Comment 17 Danielle Madeley 2005-07-02 02:26:01 UTC
Can we get this updated against the existing patch in CVS, that would save me a
heap of pain. Thanks.
Comment 18 Emmanuel Touzery 2005-07-02 06:06:34 UTC
Created attachment 48533 [details] [review]
patch updated to CVS

it's the patch against what's in CVS. the previous patch was actually a "cvs
diff", anoncvs is still not up-to-date, this patch is using
cvs.gnome.org/viewcvs
Comment 19 Kjartan Maraas 2005-07-03 17:06:10 UTC
Marking the second newest patch as obsoleted by the last one.