If you have OverDrive enabled and do a search in the OPAC, you get a link that says e.g. "Found 1 results in the library's OverDrive collection". But if you search contains non-ASCII characters, the search goes wrong. Example: Do a search for "göteborg": https://kohaopac.alingsas.se/cgi-bin/koha/opac-search.pl?q=g%C3%B6teborg (The URL shows the query as q=göteborg, but when I paste it here it is turned into q=g%C3%B6teborg.) The link says: Found 4 results in the library's OverDrive collection The actual link is: https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=g%F6teborg If you foloow that link you get to this page: https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=g%F6teborg which says "OverDrive search for 'g�teborg'" and shows 53 results, because the query was cut off at the "ö", and a search was actually made for just the letter "g" (whatever comes before the first non-ascii-char). If I change the encoding in the link from: https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=g%F6teborg to: https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=g%C3%B6teborg or https://kohaopac.alingsas.se/cgi-bin/koha/opac-overdrive-search.pl?q=göteborg the page shows 4 results, as promised.
Tested on 20.05.05 and 20.05.08. I *think* the problem is the "escape" on line 934 of koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt: $overdrive_results.html( '<a href="/cgi-bin/koha/opac-overdrive-search.pl?q=' + escape( od_querystring ) + '">' + MSG_RESULTS_FOUND_IN_COLLECTION.format(data.totalItems, 'OverDrive') + '</a>' );
"Bug 26578: Clean overdrive searches before passing to OD" touches the relevant parts of the code, but as far as I can see, it does not fix the encoding problem.
Created attachment 117194 [details] [review] Bug 27748 - Encoding problem in link to OverDrive results If you have OverDrive enabled and do a search for e.g. "Göteborg", you will get a link above the regular Koha search results that says e.g. "Found 1 results in the library's OverDrive collection". If you follow this link, a search will be made in OverDrive, but only for "G", because the search term is cut off at the first non-ASCII char. To test: - Make sure OverDrive is enabled - Do a search that contains a non-ASCII char, like "Göteborg" - Click on the link "Found x results in the library's OverDrive collection" - Verify the query string is cut off at the first non-ASCII char, and that the number of hits is different from the number given in the link on the previous page - Apply the patch, restart all the things - Repeat the search, and verify that the numbers match and that the query string is intact after clicking on the OverDrive link Lookin at the code, the same problem seems to be shared between OverDrive and RecordedBooks. I have applied the same fix to both, but I do not have access to RecordedBooks, so I have not been able to verify that the problem exists there and is fixed by my patch.
Technical note: It looks like the use of escape() has been deprecated in favour of encodeURI and encodeURIComponent: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/escape I have replaced two occurrences of escape with encodeURIComponent.
Created attachment 118481 [details] [review] Bug 27748 - Encoding problem in link to OverDrive results If you have OverDrive enabled and do a search for e.g. "Göteborg", you will get a link above the regular Koha search results that says e.g. "Found 1 results in the library's OverDrive collection". If you follow this link, a search will be made in OverDrive, but only for "G", because the search term is cut off at the first non-ASCII char. To test: - Make sure OverDrive is enabled - Do a search that contains a non-ASCII char, like "Göteborg" - Click on the link "Found x results in the library's OverDrive collection" - Verify the query string is cut off at the first non-ASCII char, and that the number of hits is different from the number given in the link on the previous page - Apply the patch, restart all the things - Repeat the search, and verify that the numbers match and that the query string is intact after clicking on the OverDrive link Lookin at the code, the same problem seems to be shared between OverDrive and RecordedBooks. I have applied the same fix to both, but I do not have access to RecordedBooks, so I have not been able to verify that the problem exists there and is fixed by my patch. Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Created attachment 119166 [details] [review] Bug 27748: Encoding problem in link to OverDrive results If you have OverDrive enabled and do a search for e.g. "Göteborg", you will get a link above the regular Koha search results that says e.g. "Found 1 results in the library's OverDrive collection". If you follow this link, a search will be made in OverDrive, but only for "G", because the search term is cut off at the first non-ASCII char. To test: - Make sure OverDrive is enabled - Do a search that contains a non-ASCII char, like "Göteborg" - Click on the link "Found x results in the library's OverDrive collection" - Verify the query string is cut off at the first non-ASCII char, and that the number of hits is different from the number given in the link on the previous page - Apply the patch, restart all the things - Repeat the search, and verify that the numbers match and that the query string is intact after clicking on the OverDrive link Lookin at the code, the same problem seems to be shared between OverDrive and RecordedBooks. I have applied the same fix to both, but I do not have access to RecordedBooks, so I have not been able to verify that the problem exists there and is fixed by my patch. Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Trusting that Magnus and Phil have tested this with access to Overdrive. Change makes sense and I will move this to PQA with a code review only.
Correct: I tested with Overdrive, and Magnus' link from comment 0 to show the problem no longer shows the problem, because he is running with his patch :)
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.05
Doesn't apply cleanly to 20.05, please rebase for backport. I'm not entirely confident in my ability to clear the merge errors correctly and don't have credentials with which to check my work with a real search. Thanks!