Bug 245759 - [PATCH] equal-sign(=) not handled in bookmarks when used in filter
Summary: [PATCH] equal-sign(=) not handled in bookmarks when used in filter
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Tools/Bookmark Manager (show other bugs)
Version: 2.3.1
Platform: Unlisted Binaries Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-26 12:40 UTC by Rindert Vonk
Modified: 2011-01-11 20:13 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.4.1
Sentry Crash Report:


Attachments
Fixed (2.44 KB, patch)
2010-07-28 17:14 UTC, Sergey Ivanov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Rindert Vonk 2010-07-26 12:40:44 UTC
Version:           2.3.1 (using KDE 4.4.5) 
OS:                Linux

Hello,

When I use this filter: rating>=8 , the result is that I see all my tracks with a rating higher or equal to 8. When I try to save this filter as a bookmark it is not saved correctly. 

Editing the url to this in the bookmark-manager does not work, the = sign does not get saved as %3D but just as =

Reproducible: Always

Steps to Reproduce:
Using a filter with = in it and try saving it as bookmark.

Actual Results:  
A bookmark with this amarok-url:
amarok://navigate/collections?filter=rating:%3E&levels=genre-artist-album&show_cover=true&show_years=true

Expected Results:  
A bookmark with this amarok-url:
amarok://navigate/collections?filter=rating:%3E%3D8&levels=genre-artist-album&show_cover=true&show_years=true

OS: Linux (x86_64) release 2.6.34-020634-generic
Compiler: cc
Comment 1 Sergey Ivanov 2010-07-28 17:14:35 UTC
Created attachment 49591 [details]
Fixed

Seems like fixed.
Comment 2 Myriam Schweingruber 2010-07-28 18:54:56 UTC
Sergey, do you mean it is already fixed in git or do you submit a patch? Your attachment looks like a patch :)
Comment 3 Sergey Ivanov 2010-07-28 20:32:16 UTC
No, i mean that this patch should fix it. BUT! I don't know amarok architecture well enough yet, so probably it (or the same stuff) used to be fixed somewhere else. This patch make to work urls only, more than that it (patch) corrects only filter arguments, so if '=' uses somewhere else (except regular application) write me back, i'll fix it too.
Comment 4 Myriam Schweingruber 2010-07-28 23:45:04 UTC
Thank you for the feedback, I will ping a developer about this.
Comment 5 Sergey Ivanov 2011-01-11 19:45:55 UTC
commit 380977f8ef617869b1a03017e5c2fae09b4ccf14
branch master
Author: Sergey Ivanov <123kash@gmail.com>
Date:   Tue Jan 11 21:45:19 2011 +0300

    Fixed missing equel-sign ('=') in filter string of bookmarks.
    BUG: 245759

diff --git a/ChangeLog b/ChangeLog
index 4c5b6aa..64e9d61 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -11,6 +11,7 @@ VERSION 2.4.0
     * Fixed some broken radio stream URLs.
 
   BUGFIXES:
+    * Fixed missing equel-sign ('=') in filter string of bookmarks. (BR 245759)
     * Fix crash on copying tracks between collection. (BR 261364)
     * Fix fetching of script data. BBC, Free Music Charts and others should work again.
       (BR 261839)
diff --git a/src/amarokurls/AmarokUrl.cpp b/src/amarokurls/AmarokUrl.cpp
index 39858a1..19ba210 100644
--- a/src/amarokurls/AmarokUrl.cpp
+++ b/src/amarokurls/AmarokUrl.cpp
@@ -124,32 +124,15 @@ bool AmarokUrl::run()
 
 QString AmarokUrl::url() const
 {
-    QString url = "amarok://";
-    url += m_command;
-    url += '/';
-    url += m_path;
+    QUrl url;
+    url.setScheme( "amarok" );
+    url.setHost( m_command );
+    url.setPath( m_path );
 
-    if ( url.endsWith( '/' ) )
-        url.chop( 1 );
+    foreach( const QString &argName, m_arguments.keys() )
+        url.addQueryItem( argName, m_arguments[argName] );
 
-    if( m_arguments.size() > 0 )
-    {
-    
-        url += '?';
-        const QStringList args = m_arguments.keys();
-        foreach( const QString &argName, args )
-        {
-            url += argName;
-            url += '=';
-            url += m_arguments.value( argName );
-            url += '&';
-        }
-        url.chop( 1 );
-    }
-
-    QUrl qUrl( url );
-
-    return QString( qUrl.toEncoded() );
+    return url.toEncoded();
 }
 
 bool AmarokUrl::saveToDb()