Summary: | History of anonymous searches is not kept | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | OPAC | Assignee: | Chris Cormack <chris> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | major | ||
Priority: | P5 - low | CC: | chris, gmcharlt, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Small patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Bug Depends on: | 6554 | ||
Bug Blocks: | |||
Attachments: |
Bug 10070: Anonymous searches are broken
Bug 10070: Anonymous search history is not being set in the cookie [PASSED QA] Bug 10070: Anonymous search history is not being set in the cookie [3.12] Bug 10070: [3.12.x] revert previous changes [master] Bug 10070: revert previous changes Signed off patch [PASSED QA] Bug 10070: revert previous changes |
Description
Jonathan Druart
2013-04-17 13:37:52 UTC
Created attachment 17499 [details] [review] Bug 10070: Anonymous searches are broken Test plan: 1 - turn on EnableOpacSearchHistory 2 - launch some searches at the opac 3 - go to your search history 4 - there is no history! 5 - apply this patch 6 - retry steps 1 to 3 7 - your history search is available! Bug title is quite misleading, you can search, just the history is not saved, testing patch now I also worry this is not the only thing the utf8 changes have broken, however they are probably too buried to revert now Created attachment 17514 [details] [review] Bug 10070: Anonymous search history is not being set in the cookie Test plan: 1 - turn on EnableOpacSearchHistory 2 - launch some searches at the opac 3 - go to your search history 4 - there is no history! 5 - apply this patch 6 - retry steps 1 to 3 7 - your history search is available! Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> This does fix the bug, but does undo the change to make the cookie utf-8 safe, however I think that change was done in the wrong way so I am happy to sign this off Created attachment 17666 [details] [review] [PASSED QA] Bug 10070: Anonymous search history is not being set in the cookie Test plan: 1 - turn on EnableOpacSearchHistory 2 - launch some searches at the opac 3 - go to your search history 4 - there is no history! 5 - apply this patch 6 - retry steps 1 to 3 7 - your history search is available! Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> This does fix the bug, but does undo the change to make the cookie utf-8 safe, however I think that change was done in the wrong way so I am happy to sign this off Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Wondering if the comment line should be deleted now too. Patch fixes the problem. This patch has been pushed to master and 3.12.x. Jared, since you reverted patches for bug 6554, this patch should be reverted from 3.12 branch too. (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. (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 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. Created attachment 17709 [details] [review] [3.12] Bug 10070: [3.12.x] revert previous changes Created attachment 17710 [details] [review] [master] Bug 10070: revert previous changes We should use escape_uri instead of escape_uri_utf8 With these 2 patches I can search several terms (with utf8 characters) and keep my anonymous history searches on 3.12 and master. Created attachment 17774 [details] [review] Signed off patch Both patches work for both master and 3.12.x so have just signed off one and obsoleted them both. 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. 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. This patch has been pushed directly to 3.12.x. (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.. Version back to master. Pushed to master. Thanks, Jonathan! |