Bug 10070 - History of anonymous searches is not kept
Summary: History of anonymous searches is not kept
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: ---
Assignee: Chris Cormack
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 6554
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-17 13:37 UTC by Jonathan Druart
Modified: 2014-12-07 20:02 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10070: Anonymous searches are broken (1.20 KB, patch)
2013-04-17 13:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10070: Anonymous search history is not being set in the cookie (1.44 KB, patch)
2013-04-17 22:07 UTC, Chris Cormack
Details | Diff | Splinter Review
[PASSED QA] Bug 10070: Anonymous search history is not being set in the cookie (1.59 KB, patch)
2013-04-25 06:30 UTC, Katrin Fischer
Details | Diff | Splinter Review
[3.12] Bug 10070: [3.12.x] revert previous changes (1009 bytes, patch)
2013-04-26 13:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
[master] Bug 10070: revert previous changes (1.03 KB, patch)
2013-04-26 13:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Signed off patch (1.32 KB, patch)
2013-04-28 19:38 UTC, Chris Cormack
Details | Diff | Splinter Review
[PASSED QA] Bug 10070: revert previous changes (1.41 KB, patch)
2013-04-29 05:20 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 Jonathan Druart 2013-04-17 13:37:52 UTC
If no user is connected at the OPAC, and the pref EnableOpacSearchHistory is turn on, the history search should be accessible. It is not the case in master.

git bisect says me that this commit is guilty:

d542740ab8877234d043c8a11cb19a6004e72656 is the first bad commit
commit d542740ab8877234d043c8a11cb19a6004e72656
Author: Dobrica Pavlinusic <dpavlin@rot13.org>
Date:   Sun Jun 10 15:18:42 2012 +0100

    Bug 6554 - make Koha internally utf-8 clean
Comment 1 Jonathan Druart 2013-04-17 13:40:33 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2013-04-17 22:02:03 UTC
Bug title is quite misleading, you can search, just the history is not saved, testing patch now
Comment 3 Chris Cormack 2013-04-17 22:04:03 UTC
I also worry this is not the only thing the utf8 changes have broken, however they are probably too buried to revert now
Comment 4 Chris Cormack 2013-04-17 22:07:45 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2013-04-25 06:30:24 UTC Comment hidden (obsolete)
Comment 6 Jared Camins-Esakov 2013-04-26 00:36:03 UTC
This patch has been pushed to master and 3.12.x.
Comment 7 Jonathan Druart 2013-04-26 07:14:56 UTC
Jared, since you reverted patches for bug 6554, this patch should be reverted from 3.12 branch too.
Comment 8 Jared Camins-Esakov 2013-04-26 11:45:13 UTC
(In reply to comment #7)
> Jared, since you reverted patches for bug 6554, this patch should be
> reverted from 3.12 branch too.

Are you sure? If I revert this patch the search history ceases to function.
Comment 9 Jonathan Druart 2013-04-26 11:54:55 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > Jared, since you reverted patches for bug 6554, this patch should be
> > reverted from 3.12 branch too.
> 
> Are you sure? If I revert this patch the search history ceases to function.

Ho yes sorry, you are right.
However there is an issue:
1/ logout
2/ search "bar" (added to the history)
3/ search "féé" => history is empty
Comment 10 Jonathan Druart 2013-04-26 13:43:41 UTC
Whaou, in fact the solution is easy, the function to call is uri_escape (what it was before bug 6554).

uri_escape_utf8 was introduced by bug 6554 and this patch fixed the issue but not for utf8 characters. So in 3.12, if you revert this one, it will introduce the uri_escape_utf8 call :)

So I will provide 1 patch for 3.12 and 1 for master.
I think it is not useful to open a new report, say me if you disagree.
Comment 11 Jonathan Druart 2013-04-26 13:44:54 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2013-04-26 13:46:42 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2013-04-26 13:48:41 UTC
With these 2 patches I can search several terms (with utf8 characters) and keep my anonymous history searches on 3.12 and master.
Comment 14 Chris Cormack 2013-04-28 19:38:12 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2013-04-29 05:20:28 UTC
Created attachment 17779 [details] [review]
[PASSED QA] Bug 10070: revert previous changes

To test
1/ logout
2/ search "bar" (added to the history)
3/ search "féé" => history is empty

Apply patch
1/ logout
2/ search "bar" (added to the history)
3/ search "féé" => added to the history

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes tests and QA script.
Comment 16 Jared Camins-Esakov 2013-05-02 02:39:32 UTC
This patch only works on 3.12.x for me, even after 6554 has been reverted from master. I am going to push it to master, and defer a decision about what to do with this bug in master to the 3.14 RM.
Comment 17 Jared Camins-Esakov 2013-05-02 02:40:41 UTC
This patch has been pushed directly to 3.12.x.
Comment 18 Marcel de Rooy 2013-05-02 09:47:02 UTC
(In reply to comment #16)
> This patch only works on 3.12.x for me, even after 6554 has been reverted
> from master. I am going to push it to master, and defer a decision about
> what to do with this bug in master to the 3.14 RM.

The binmode line in C4/Output.pm makes the difference in sub output_html_with_http_headers.
Remove it from master.
I wasn't too happy with a binmode there anyway..
Comment 19 Marcel de Rooy 2013-05-23 08:27:32 UTC
Version back to master.
Comment 20 Galen Charlton 2013-07-25 15:56:57 UTC
Pushed to master.  Thanks, Jonathan!