Bug 10578

Summary:	Escaping CGI breaks results sort feature (librarian's interface)
Product:	Koha	Reporter:	Janusz Kaczmarek <januszop>
Component:	Searching	Assignee:	Janusz Kaczmarek <januszop>
Status:	CLOSED FIXED	QA Contact:
Severity:	normal
Priority:	P5 - low	CC:	gmcharlt, srdjan, tomascohen
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Proposed patch [SIGNED-OFF] Bug 10578 - Escaping CGI breaks results sort feature (librarian's interface) [PASSED QA] Bug 10578 - Escaping CGI breaks results sort feature (librarian's interface)

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)

Created attachment 19581 [details] [review]
Proposed patch

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.

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)

Created attachment 19746 [details] [review]
[SIGNED-OFF] 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>

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 (tcohen) 2013-08-16 13:25:04 UTC

This patch has been pushed to 3.12.x, will be in 3.12.4.

Thanks Janusz!