Bug 23475 - Search context is lost when simple search leads to a single record
Summary: Search context is lost when simple search leads to a single record
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Josef Moravec
URL:
Keywords:
Depends on:
Blocks: 27697
  Show dependency treegraph
 
Reported: 2019-08-19 19:38 UTC by Nick Clemens
Modified: 2021-02-15 14:54 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.05.00, 20.11.01, 20.05.07


Attachments
Bug 23475: Use cookies to preserve simple search values (3.57 KB, patch)
2019-08-19 19:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23475: Use cookies to preserve simple search values (3.76 KB, patch)
2019-10-03 11:31 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Little issue with the final é (17.51 KB, image/png)
2019-10-03 11:34 UTC, Séverine Queune
Details
Bug 23475: Use cookies to preserve simple search values (4.34 KB, patch)
2020-08-12 14:36 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23475: Make cookie name more explicit (1.72 KB, patch)
2020-09-03 13:21 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23475: (follow-up) Include URI::Escape (622 bytes, patch)
2020-10-07 11:38 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23475: Use cookies to preserve simple search values (4.40 KB, patch)
2020-10-07 15:16 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23475: Make cookie name more explicit (1.79 KB, patch)
2020-10-07 15:16 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23475: (follow-up) Include URI::Escape (688 bytes, patch)
2020-10-07 15:16 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 23475: Use cookies to preserve simple search values (4.46 KB, patch)
2020-10-15 21:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23475: Make cookie name more explicit (1.85 KB, patch)
2020-10-15 21:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23475: (follow-up) Include URI::Escape (746 bytes, patch)
2020-10-15 21:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23475: Use localStorage to preserve simple search values (4.56 KB, patch)
2020-10-30 13:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23475: Use localStorage to preserve simple search values (4.88 KB, patch)
2020-10-30 15:28 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23475: Use localStorage to preserve simple search values (4.92 KB, patch)
2020-11-04 02:07 UTC, David Nind
Details | Diff | Splinter Review
Bug 23475: Use localStorage to preserve simple search values (4.98 KB, patch)
2020-11-17 16:47 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 23475: (follow-up) Fix selectors and stray use statements (1.87 KB, patch)
2020-11-18 13:55 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23475: Unset the storage if the values do not exist (1.47 KB, patch)
2020-11-20 19:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23475: Use localStorage to preserve simple search values (4.43 KB, patch)
2020-11-30 10:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23475: Unset the storage if the values do not exist (1.47 KB, patch)
2020-11-30 10:06 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-08-19 19:38:34 UTC
Bug 19046 made search terms and dropdown preserve in the staff client, however, if one search for a barcode or something that returns a single record the dropdown and box are cleared
Comment 1 Nick Clemens 2019-08-19 19:39:32 UTC
Created attachment 92357 [details] [review]
Bug 23475: Use cookies to preserve simple search values
Comment 2 Nick Clemens 2019-08-19 19:40:58 UTC
This patch is a POC - it needs some work and extension for the OPAC, but it should demonstrate possibility of passing through to the details page - Other options would be to build everythign into the url, or be able to process details page from search.pl - this seemed a quick way to get things there
Comment 3 sandboxes@biblibre.com 2019-10-03 11:31:10 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 4 sandboxes@biblibre.com 2019-10-03 11:31:31 UTC
Created attachment 93558 [details] [review]
Bug 23475: Use cookies to preserve simple search values

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 5 Séverine Queune 2019-10-03 11:33:46 UTC
Finally worked cleaning the cookies !
I just had a little encoding issue with the last 'é' (see screenshot) but I don't know it if comes from the patch I'm not sure it comes form the patch so I signed it.
Comment 6 Séverine Queune 2019-10-03 11:34:46 UTC
Created attachment 93562 [details]
Little issue with the final é
Comment 7 Marcel de Rooy 2019-10-11 10:31:27 UTC
Severine: Please describe how you got the encoding issue. I see a title field on your image. Where does it come from ?

Nick: At first glance it seems to me that you should encode/decode unicode stuff when saving and fetching from CGI cookies. I had a quick look in CGI on cpan but the docs are very quiet about it. Cannot see if -utf8 should handle cookie values too. Please check.

Changing status for feedback
Comment 8 Séverine Queune 2019-10-11 11:30:49 UTC
Hi Marcel,

I know I used a Biblibre's sandbox in Unimarc, but I can't remember if it was  http://pro.es5-koha.sandbox.biblibre.eu (login with test/test) which uses ES5 or http://pro.es6-koha.sandbox.biblibre.eu with ES6.

I tried with several title and everything worked fine, except for the title "Mademoiselle liberté" with this little issue.
This is something quite usual with some letter with diacritics in French (see for example bug 23542), so as I said, I'm not sure it comes from the patch, nor if the patch could solve this problem.
Comment 9 Nick Clemens 2020-08-12 14:36:51 UTC
Created attachment 108124 [details] [review]
Bug 23475: Use cookies to preserve simple search values

To test:
1 - Enable IntranetCatalogSearchPulldown
2 - Perform a search that brings you to a single title
3 - Note the search is not displayed on the details page
4 - Apply patch
5 - Repeat
6 - Note the search is retained on details page
7 - Try with different dropdown values and search terms
8 - Test search terms including accented characters etc
Comment 10 Nick Clemens 2020-08-12 14:38:34 UTC
(In reply to Marcel de Rooy from comment #7)
> Nick: At first glance it seems to me that you should encode/decode unicode
> stuff when saving and fetching from CGI cookies. I had a quick look in CGI
> on cpan but the docs are very quiet about it. Cannot see if -utf8 should
> handle cookie values too. Please check.

It looks like Firefox and Chrome both URI encode the values when we save them.

To decode we need unescape, then utf8 decode, at least that's what worked.

Afaict, things are not consistent:
https://stackoverflow.com/questions/1969232/what-are-allowed-characters-in-cookies
Comment 11 Fridolin Somers 2020-09-03 12:47:42 UTC
Nice work.
In my opinion cookie name "pulldown_selection" is too generic.
I propose you use a prefix like "cat_search".
Comment 12 Nick Clemens 2020-09-03 13:21:48 UTC
Created attachment 109624 [details] [review]
Bug 23475: Make cookie name more explicit
Comment 13 Sally 2020-10-06 12:10:53 UTC
I loaded this onto a sandbox, but received the following error when searching for a record with just one match:

Undefined subroutine &CGI::Compile::ROOT::kohadevbox_koha_catalogue_detail_2epl::uri_unescape called at /kohadevbox/koha/catalogue/detail.pl line 121

A search which returns multiple records functions as expected.
Comment 14 Nick Clemens 2020-10-07 11:38:18 UTC
Created attachment 111325 [details] [review]
Bug 23475: (follow-up) Include URI::Escape
Comment 15 ByWater Sandboxes 2020-10-07 15:16:03 UTC
Created attachment 111338 [details] [review]
Bug 23475: Use cookies to preserve simple search values

To test:
1 - Enable IntranetCatalogSearchPulldown
2 - Perform a search that brings you to a single title
3 - Note the search is not displayed on the details page
4 - Apply patch
5 - Repeat
6 - Note the search is retained on details page
7 - Try with different dropdown values and search terms
8 - Test search terms including accented characters etc

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 16 ByWater Sandboxes 2020-10-07 15:16:07 UTC
Created attachment 111339 [details] [review]
Bug 23475: Make cookie name more explicit

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 17 ByWater Sandboxes 2020-10-07 15:16:11 UTC
Created attachment 111340 [details] [review]
Bug 23475: (follow-up) Include URI::Escape

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 18 Sally 2020-10-07 15:18:14 UTC
Works well - here's some of the accented searches I used on the sandbox in case they're useful for others testing:

Howe, Zoë Street
Strk̆v̜elser
Ślez
Comment 19 Katrin Fischer 2020-10-15 21:05:35 UTC
Created attachment 111798 [details] [review]
Bug 23475: Use cookies to preserve simple search values

To test:
1 - Enable IntranetCatalogSearchPulldown
2 - Perform a search that brings you to a single title
3 - Note the search is not displayed on the details page
4 - Apply patch
5 - Repeat
6 - Note the search is retained on details page
7 - Try with different dropdown values and search terms
8 - Test search terms including accented characters etc

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Katrin Fischer 2020-10-15 21:05:40 UTC
Created attachment 111799 [details] [review]
Bug 23475: Make cookie name more explicit

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Katrin Fischer 2020-10-15 21:05:45 UTC
Created attachment 111800 [details] [review]
Bug 23475: (follow-up) Include URI::Escape

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Katrin Fischer 2020-10-15 21:06:51 UTC
Please document the new cookies here:
https://wiki.koha-community.org/wiki/Use_of_Cookies
Comment 23 Jonathan Druart 2020-10-25 22:42:02 UTC
Why a cookie? We could use localStorage here.
Comment 24 Nick Clemens 2020-10-30 13:52:46 UTC
Created attachment 112697 [details] [review]
Bug 23475: Use localStorage to preserve simple search values

To test:
1 - Enable IntranetCatalogSearchPulldown
2 - Perform a search that brings you to a single title
3 - Note the search is not displayed on the details page
4 - Apply patch
5 - Repeat
6 - Note the search is retained on details page
7 - Try with different dropdown values and search terms
8 - Test search terms including accented characters etc
Comment 25 Nick Clemens 2020-10-30 15:28:10 UTC
Created attachment 112714 [details] [review]
Bug 23475: Use localStorage to preserve simple search values

To test:
1 - Enable IntranetCatalogSearchPulldown
2 - Perform a search that brings you to a single title
3 - Note the search is not displayed on the details page
4 - Apply patch
5 - Repeat
6 - Note the search is retained on details page
7 - Try with different dropdown values and search terms
8 - Test search terms including accented characters etc
Comment 26 David Nind 2020-11-04 02:07:05 UTC
Created attachment 112971 [details] [review]
Bug 23475: Use localStorage to preserve simple search values

To test:
1 - Enable IntranetCatalogSearchPulldown
2 - Perform a search that brings you to a single title
3 - Note the search is not displayed on the details page
4 - Apply patch
5 - Repeat
6 - Note the search is retained on details page
7 - Try with different dropdown values and search terms
8 - Test search terms including accented characters etc

Signed-off-by: David Nind <david@davidnind.com>
Comment 27 Josef Moravec 2020-11-17 16:47:53 UTC
Created attachment 113744 [details] [review]
Bug 23475: Use localStorage to preserve simple search values

To test:
1 - Enable IntranetCatalogSearchPulldown
2 - Perform a search that brings you to a single title
3 - Note the search is not displayed on the details page
4 - Apply patch
5 - Repeat
6 - Note the search is retained on details page
7 - Try with different dropdown values and search terms
8 - Test search terms including accented characters etc

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 28 Jonathan Druart 2020-11-18 08:42:19 UTC
1. There are 2 unneeded use statements in catalogue/detail.pl
2. Shouldn't we use the same selectors for:

+            if ( search_index ){ $('.advsearch[name="idx"]').val(search_index)};
vs
+    localStorage.setItem('cat_search_pulldown_selection', $("#cat-search-block select.advsearch").val() );

 and

+            if ( search_value ){ $('#search-form').val(search_value)};
vs
+    localStorage.setItem('searchbox_value', $("#cat-search-block #search-form").val() );

to make thing more obvious. Is there a reason not to?
Comment 29 Nick Clemens 2020-11-18 13:55:34 UTC
Created attachment 113784 [details] [review]
Bug 23475: (follow-up) Fix selectors and stray use statements
Comment 30 Jonathan Druart 2020-11-19 13:33:08 UTC
Found a bug:
1. Search for "Chapman" on the adv search
2. The detail page has "undefined" in the search box
Comment 31 Nick Clemens 2020-11-20 19:54:14 UTC
Created attachment 113904 [details] [review]
Bug 23475: Unset the storage if the values do not exist
Comment 32 Jonathan Druart 2020-11-30 10:06:46 UTC
Created attachment 114055 [details] [review]
Bug 23475: Use localStorage to preserve simple search values

To test:
1 - Enable IntranetCatalogSearchPulldown
2 - Perform a search that brings you to a single title
3 - Note the search is not displayed on the details page
4 - Apply patch
5 - Repeat
6 - Note the search is retained on details page
7 - Try with different dropdown values and search terms
8 - Test search terms including accented characters etc

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Bug 23475: (follow-up) Fix selectors and stray use statements
Comment 33 Jonathan Druart 2020-11-30 10:06:51 UTC
Created attachment 114056 [details] [review]
Bug 23475: Unset the storage if the values do not exist
Comment 34 Jonathan Druart 2020-11-30 10:21:28 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 35 Fridolin Somers 2020-11-30 10:53:22 UTC
Pushed to 20.11.x for 20.11.01
Comment 36 Andrew Fuerste-Henry 2020-12-15 20:15:35 UTC
Backported to 20.05.x for 20.05.07
Comment 37 Victor Grousset/tuxayo 2020-12-17 00:28:29 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.