Bug 21950 - Searching with 'accents' breaks on navigating to the second page of results
Summary: Searching with 'accents' breaks on navigating to the second page of results
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 21526 21878
Blocks:
  Show dependency treegraph
 
Reported: 2018-12-05 11:33 UTC by victor
Modified: 2019-01-02 09:43 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21950: Remove filtering for query_cgi and limit_cgi (24.51 KB, patch)
2018-12-06 13:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21950: Remove filtering for query_cgi and limit_cgi (24.51 KB, patch)
2018-12-10 17:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21950: Remove filtering for query_cgi and limit_cgi (24.56 KB, patch)
2018-12-13 15:57 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description victor 2018-12-05 11:33:18 UTC
when we search for a term with accents in opac the first page appears normal, but when we click to go to next page it presents an error.
Comment 1 Owen Leonard 2018-12-05 12:55:47 UTC
I can confirm this in master. The accented character gets encoded in the pagination links in such a way that it breaks the search.

é in the search string looks like %25C3%25A9
Comment 2 Jonathan Druart 2018-12-06 13:06:37 UTC
We must not escape query_cgi and limit_cgi template-side, they are already escape properly from build_query_compat using uri_escape_utf8.

To fix further problems we should replace all occurrences to make things clear (I decided to keep the html filter so far, which did not hurt, but uri or url do)

Same patch as the following commit will be provided
commit 2fc599c0893620c395ca0492c9d9e3c860c8f951
    Bug 21526: Fix search result pages (url vs uri vs raw)

    query_cgi is uri_escaped from the pl, so we should displayed as raw

    Test plan:
    Use wide characters ❤
    Search, filter, facets, search history, rss (both interfaces)
Comment 3 Jonathan Druart 2018-12-06 13:09:01 UTC
Created attachment 82907 [details] [review]
Bug 21950: Remove filtering for query_cgi and limit_cgi

We must not escape query_cgi and limit_cgi template-side, they are already
escape properly from build_query_compat using uri_escape_utf8.

To fix further problems we should replace all occurrences to make things
clear (I decided to keep the html filter so far, which did not hurt, but uri or url do)

Same patch as the following commit will be provided
commit 2fc599c0893620c395ca0492c9d9e3c860c8f951
    Bug 21526: Fix search result pages (url vs uri vs raw)

    query_cgi is uri_escaped from the pl, so we should displayed as raw

    Test plan:
    Use wide characters ❤
    Search, filter, facets, search history, rss (both interfaces)
Comment 4 Jonathan Druart 2018-12-06 13:11:33 UTC
***Please test deeply!***
Comment 5 victor 2018-12-06 15:25:53 UTC
Will the issue be fixed in the next update?
Comment 6 Owen Leonard 2018-12-10 17:59:46 UTC
Created attachment 83021 [details] [review]
Bug 21950: Remove filtering for query_cgi and limit_cgi

We must not escape query_cgi and limit_cgi template-side, they are already
escape properly from build_query_compat using uri_escape_utf8.

To fix further problems we should replace all occurrences to make things
clear (I decided to keep the html filter so far, which did not hurt, but uri or url do)

Same patch as the following commit will be provided
commit 2fc599c0893620c395ca0492c9d9e3c860c8f951
    Bug 21526: Fix search result pages (url vs uri vs raw)

    query_cgi is uri_escaped from the pl, so we should displayed as raw

    Test plan:
    Use wide characters ❤
    Search, filter, facets, search history, rss (both interfaces)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 7 Martin Renvoize 2018-12-13 15:57:27 UTC
Created attachment 83177 [details] [review]
Bug 21950: Remove filtering for query_cgi and limit_cgi

We must not escape query_cgi and limit_cgi template-side, they are already
escape properly from build_query_compat using uri_escape_utf8.

To fix further problems we should replace all occurrences to make things
clear (I decided to keep the html filter so far, which did not hurt, but uri or url do)

Same patch as the following commit will be provided
commit 2fc599c0893620c395ca0492c9d9e3c860c8f951
    Bug 21526: Fix search result pages (url vs uri vs raw)

    query_cgi is uri_escaped from the pl, so we should displayed as raw

    Test plan:
    Use wide characters ❤
    Search, filter, facets, search history, rss (both interfaces)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2018-12-13 15:57:50 UTC
Can't find any regressions.. Passing QA
Comment 9 Nick Clemens 2018-12-14 15:34:26 UTC
Awesome work all!

Pushed to master for 19.05
Comment 10 Martin Renvoize 2018-12-17 12:03:00 UTC
Pushed to 18.11.x for 18.11.01
Comment 11 Lucas Gass 2018-12-24 15:34:17 UTC
requires dependencies wont backport to 18.05