Bug 249376 - [PATCH][Windows] Amarok crashes when exporting playlist to any file type.
Summary: [PATCH][Windows] Amarok crashes when exporting playlist to any file type.
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Playlist (show other bugs)
Version: 2.3.1-GIT
Platform: Compiled Sources Microsoft Windows
: NOR crash
Target Milestone: 2.4.0
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-29 03:32 UTC by Dan
Modified: 2010-09-27 22:58 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.4


Attachments
The above mentioned patch. (905 bytes, patch)
2010-09-09 09:42 UTC, James Duncan
Details
The modified patch (1.66 KB, patch)
2010-09-09 09:59 UTC, James Duncan
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dan 2010-08-29 03:32:53 UTC
Version:           2.3-GIT (using Devel) 
OS:                MS Windows



Reproducible: Always

Steps to Reproduce:
Export playlist to a file.

Actual Results:  
Amarok stops responding and the bug reported appears

Expected Results:  
To save my playlist to a file of my choice, in my location of choice.

OS: WindowsNT (i686) release Windows 7
Compiler: cl.exe
Comment 1 Martin Blumenstingl 2010-08-30 21:45:23 UTC
Hi,

could you please post the backtrace from the crash?
Many developers don't have a windows box where they could test this
Comment 2 Myriam Schweingruber 2010-08-30 22:02:40 UTC
Setting status correctly
Comment 3 Myriam Schweingruber 2010-08-31 10:00:02 UTC
Closing then :)
Comment 4 James Duncan 2010-09-09 09:42:36 UTC
Created attachment 51451 [details]
The above mentioned patch.
Comment 5 James Duncan 2010-09-09 09:44:37 UTC
Oops...  Did that backwards...  Anyways, the bug looks to be caused by a colon in the default file name for new playlists; since NTFS and FAT don't like colon's, this would cause a failure/crash on write.  I don't have a windows build, but the attached patch should do the trick (changed the colon to a hyphen).
Comment 6 James Duncan 2010-09-09 09:59:13 UTC
Modified patch that additionally fixes one more possible problem (in mediadevicecollection).
Comment 7 James Duncan 2010-09-09 09:59:47 UTC
Created attachment 51453 [details]
The modified patch
Comment 8 Myriam Schweingruber 2010-09-09 10:08:22 UTC
James, thank you for your patch, but what I don't get is: Dan already fixed that 9 days ago in his installer...
Comment 9 James Duncan 2010-09-10 07:24:32 UTC
Even if it was fixed in the Windows installer, the bug still appears to be present in a GIT build on a Unix system.  If the user's home directory is on an NTFS or FAT partition, then exporting a playlist will fail because the filename contains a colon; this colon still exists in the filename in the most current GIT, and that's what my patch fixes.
Comment 10 Myriam Schweingruber 2010-09-10 15:02:44 UTC
Well, then it is not fixed, reopening.

Dan, patches need to be applied to the git version, else this will not be consistent.
Comment 11 Myriam Schweingruber 2010-09-10 15:03:54 UTC
Confirmed by James.
Comment 12 Mark Kretschmann 2010-09-27 12:48:53 UTC
commit e17ac0484fa8b5c7cfec7c3f79e4a339b451b8ae
Author: Mark Kretschmann <kretschmann@kde.org>
Date:   Mon Sep 27 12:47:57 2010 +0200

    commit 141c5b25aaecbd7204878c102285dc47c5455d6a
    Author: James Duncan <james.t.duncan@gmail.com>
    Date:   Thu Sep 9 00:39:45 2010 -0700
    
    Changed colon to hyphen in format string for filename of playlists.
    
    BUG:249376

diff --git a/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.cpp b/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.cpp
index e31579f..12d3b46 100644
--- a/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.cpp
+++ b/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.cpp
@@ -150,7 +150,7 @@ MediaDeviceUserPlaylistProvider::save( const Meta::TrackList &tracks )
             filteredTracks << track;
 
     return save( filteredTracks,
-                 QDateTime::currentDateTime().toString( "ddd MMMM d yy hh:mm" ) );
+                 QDateTime::currentDateTime().toString( "ddd MMMM d yy hh-mm" ) );
 }
 
 Playlists::PlaylistPtr
diff --git a/src/playlistmanager/file/PlaylistFileProvider.cpp b/src/playlistmanager/file/PlaylistFileProvider.cpp
index 01e2325..9200ec3 100644
--- a/src/playlistmanager/file/PlaylistFileProvider.cpp
+++ b/src/playlistmanager/file/PlaylistFileProvider.cpp
@@ -206,7 +206,7 @@ PlaylistFileProvider::trackActions( Playlists::PlaylistPtr playlist, int trackIn
 Playlists::PlaylistPtr
 PlaylistFileProvider::save( const Meta::TrackList &tracks )
 {
-    return save( tracks, QDateTime::currentDateTime().toString( "ddd MMMM d yy hh:mm") + ".xspf" );
+    return save( tracks, QDateTime::currentDateTime().toString( "ddd MMMM d yy hh-mm") + ".xspf" );
 }
 
 Playlists::PlaylistPtr