Bug 111614 - adding folder from media:/hda5 gives error
Summary: adding folder from media:/hda5 gives error
Status: RESOLVED FIXED
Alias: None
Product: juk
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Scott Wheeler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-08-27 21:30 UTC by Mohd Asif Ali Rizwaan
Modified: 2005-09-03 01:28 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
First pass at implementing support. (1.92 KB, patch)
2005-08-31 01:09 UTC, Michael Pyne
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mohd Asif Ali Rizwaan 2005-08-27 21:30:16 UTC
Version:           2.3 (using KDE 3.4.90 (alpha1, >= 20050806), compiled sources)
Compiler:          gcc version 3.3.4
OS:                Linux (i686) release 2.6.11.7

hi,

I have Songs folder in /mnt/d/Songs (or media:/hda5/Songs), adding from media:/hda5 causes error.

perhaps media:/ ioslave is not supported by juk, kindly support it for ease of use. thanks.
Comment 1 Michael Pyne 2005-08-30 05:01:05 UTC
JuK itself only supports local files, which should be OK in this case since a media:/ link is really a local file.

There is supposed to be a method to resolve these references (using KFileItem::localPath()) but it doesn't seem to work.  I would rather have it fixed in KIO or KFilePath than try to hack around it in JuK.
Comment 2 Thiago Macieira 2005-08-30 11:40:07 UTC
It is fixed in KIO and KFileItem: just use mostLocalURL (http://developer.kde.org/documentation/library/cvs-api/kdelibs-apidocs/kio/kio/html/classKFileItem.html#a55).
Comment 3 Michael Pyne 2005-08-31 01:09:58 UTC
Created attachment 12429 [details]
First pass at implementing support.

Thiago:

I tried the attached patch but both home:/ and media:/ urls don't work (easiest
way is to use "dcop juk Collection openFile media:/path/to/file" to test).  I
get the following output:

juk: Mapping media:/sda3/home/kde-cvs/music/goat-CV-Scourge-DoD.mp3 to
/sda3/home/kde-cvs/music/goat-CV-Scourge-DoD.mp3
juk: ERROR: media:/sda3/home/kde-cvs/music/goat-CV-Scourge-DoD.mp3 is not a
local file, JuK can only use local files.

home:/ URLs are equally broken.
Comment 4 Thiago Macieira 2005-09-01 21:36:14 UTC
Any help, Kévin?
Comment 5 Kevin Ottens 2005-09-02 10:16:10 UTC
+ KFileItem kfi(KFileItem::Unknown, KFileItem::Unknown, file, true /* load mime on demand */);

It's the problem actually. Creating a KFileItem like this doesn't work
since you have no guarantee that it'll be stated (and then the UDS_LOCAL_PATH)
discovered. I strongly advise you to use the convenience NetAccess::mostLocalURL() method which does all the work for you.
Comment 6 Michael Pyne 2005-09-03 01:19:07 UTC
Reporter: How were you able to come across this issue?  I have a fix which I am fixing to commit but I've been unable to actually get JuK to try to add a media:/ or home:/ URL.  The Add Folder dialog doesn't allow URLs to be entered, and the Open Files dialog is restricted to local files only.

Also, drag-and-drop from Konqueror seems to automatically get converted to the filename, so that doesn't trip the bug either.

The only path I see remaining is through DCOP, but the API is quite clear on it only adding files. ;)
Comment 7 Michael Pyne 2005-09-03 01:28:08 UTC
SVN commit 456471 by mpyne:

Fix bug 111614 (JuK doesn't support media:/ ioslave).  There's a catch though, you still
won't be able to add files from media:/ from within JuK, until they fix the KFileDialog to
either return media:/ URLs or convert them to local files when in LocalFileOnly mode.

You can drag them in from Konqueror though, although Konq already converts the filenames so
I don't see how that was an issue.

So in short, I fixed a bug that you shouldn't be able to reproduce... weird.  But this helps
to future-proof JuK a little.

BUG:111614


 M  +20 -1     mediafiles.cpp  
 M  +11 -0     mediafiles.h  
 M  +1 -1      playlist.cpp  


--- branches/KDE/3.5/kdemultimedia/juk/mediafiles.cpp #456470:456471
@@ -14,7 +14,9 @@
  ***************************************************************************/
 
 #include <kfiledialog.h>
+#include <kdebug.h>
 #include <klocale.h>
+#include <kio/netaccess.h>
 
 #include "mediafiles.h"
 
@@ -55,7 +57,7 @@
 
     dialog.exec();
 
-    return dialog.selectedFiles();
+    return convertURLsToLocal(dialog.selectedFiles());
 }
 
 QString MediaFiles::savePlaylistDialog(const QString &playlistName, QWidget *parent)
@@ -137,3 +139,20 @@
     ;
     return l;
 }
+
+QStringList MediaFiles::convertURLsToLocal(const QStringList &urlList, QWidget *w)
+{
+    QStringList result;
+    KURL localUrl;
+
+    for(QStringList::ConstIterator it = urlList.constBegin(); it != urlList.constEnd(); ++it) {
+	localUrl = KIO::NetAccess::mostLocalURL(KURL::fromPathOrURL(*it), w);
+
+	if(!localUrl.isLocalFile())
+	    kdDebug(65432) << localUrl << " is not a local file, skipping.\n";
+	else
+	    result.append(localUrl.path());
+    }
+
+    return result;
+}
--- branches/KDE/3.5/kdemultimedia/juk/mediafiles.h #456470:456471
@@ -73,6 +73,17 @@
      * Returns true if fileName is an Ogg/FLAC file.
      */
     bool isOggFLAC(const QString &fileName);
+
+    /**
+     * Returns a list of absolute local filenames, mapped from \p urlList.
+     * Any URLs in urlList that aren't really local files will be stripped
+     * from the result (so result.size() may be < urlList.size()).
+     *
+     * @param urlList list of file names or URLs to convert.
+     * @param w KIO may need the widget to handle user interaction.
+     * @return list of all local files in urlList, converted to absolute paths.
+     */
+    QStringList convertURLsToLocal(const QStringList &urlList, QWidget *w = 0);
 }
 
 #endif
--- branches/KDE/3.5/kdemultimedia/juk/playlist.cpp #456470:456471
@@ -1078,7 +1078,7 @@
     QStringList fileList;
 
     for(KURL::List::Iterator it = urls.begin(); it != urls.end(); ++it)
-	fileList.append((*it).path());
+	fileList += MediaFiles::convertURLsToLocal((*it).path(), this);
 
     addFiles(fileList, item);
 }