Bug 10578 - Escaping CGI breaks results sort feature (librarian's interface)
Summary: Escaping CGI breaks results sort feature (librarian's interface)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Janusz Kaczmarek
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-11 17:13 UTC by Janusz Kaczmarek
Modified: 2014-12-07 20:02 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (2.27 KB, patch)
2013-07-11 17:18 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10578 - Escaping CGI breaks results sort feature (librarian's interface) (2.41 KB, patch)
2013-07-18 04:45 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
[PASSED QA] Bug 10578 - Escaping CGI breaks results sort feature (librarian's interface) (2.51 KB, patch)
2013-07-21 11:01 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2013-07-11 17:13:24 UTC
After introducing escaped $query_cgi in bugfix 9239 the results sort feature does not work correctly for the librarian. 

To reproduce the problem (in librarian's interface):

1. Have some biblio records to get a list when searching
2. Make an advanced search with 'as phrase' modifier (title -- ti,phr: or author -- au,phr: , etc.)
3. Having the result list try to change the sort order.
4. You should get no results with a message like: "No results match your search for 'au%2Cphr: " etc.

A patch will follow.
Comment 1 Janusz Kaczmarek 2013-07-11 17:18:14 UTC Comment hidden (obsolete)
Comment 2 Srdjan Jankovic 2013-07-17 03:43:03 UTC
Why not
my @operators = map uri_unescape($_), $cgi->param('op');

etc
Comment 3 Janusz Kaczmarek 2013-07-17 06:15:08 UTC
(In reply to Srdjan Jankovic from comment #2)
> Why not
> my @operators = map uri_unescape($_), $cgi->param('op');
> 
> etc

Just to be consistent with how it looks like in analogous spots in opac/opac-search.pl...

But feel free to modify if you think it is worth of.
Comment 4 Srdjan Jankovic 2013-07-18 04:45:50 UTC Comment hidden (obsolete)
Comment 5 Srdjan Jankovic 2013-07-18 04:46:50 UTC
It is. I hate wasted CPU cycles.
Comment 6 Katrin Fischer 2013-07-21 11:01:44 UTC
Created attachment 19834 [details] [review]
[PASSED QA] Bug 10578 - Escaping CGI breaks results sort feature (librarian's interface)

To test  (in librarian's interface):

1. Have some biblio records to get a list when searching.
2. Make an advanced search with 'as phrase' modifier (title -- ti,phr:
or author -- au,phr: , etc.)
3. Having the result list try to change the sort order.
4. You should get no results with a message like: "No results match your
search for 'au%2Cphr: " etc.
5. Apply the patch
6. Repeat steps 2. & 3.
7. You should get the list sorted according to your choice.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works nicely, passes all tests and QA script.
Comment 7 Galen Charlton 2013-07-22 15:53:41 UTC
Pushed to master.  Thanks, Janusz!
Comment 8 Tomás Cohen Arazi 2013-08-16 13:25:04 UTC
This patch has been pushed to 3.12.x, will be in 3.12.4.

Thanks Janusz!