Bug 33734 - Using custom search filters breaks diacritics characters in search term
Summary: Using custom search filters breaks diacritics characters in search term
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: David Cook
QA Contact: Katrin Fischer
URL:
Keywords:
: 34192 (view as bug list)
Depends on: 17170
Blocks:
  Show dependency treegraph
 
Reported: 2023-05-15 11:49 UTC by Ville Korjus
Modified: 2023-10-12 16:11 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.04,22.11.11


Attachments
Bug 33734: Use raw filters with search filter links in facets.inc (2.08 KB, patch)
2023-05-23 07:24 UTC, Ville Korjus
Details | Diff | Splinter Review
Bug 33734: Use raw filters with search filter links in facets.inc (2.12 KB, patch)
2023-05-24 20:03 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 33734: Remove incorrect URL escaping on search filters (2.02 KB, patch)
2023-09-13 02:27 UTC, David Cook
Details | Diff | Splinter Review
Bug 33734: Remove incorrect URL escaping on search filters (2.07 KB, patch)
2023-09-13 06:16 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 33734: Remove incorrect URL escaping on search filters (3.66 KB, patch)
2023-09-13 07:02 UTC, David Cook
Details | Diff | Splinter Review
Bug 33734: Remove incorrect URL escaping on search filters (2.41 KB, patch)
2023-09-13 07:38 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 33734: Remove incorrect URL escaping on search filters (3.77 KB, patch)
2023-09-15 14:48 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 Ville Korjus 2023-05-15 11:49:53 UTC
To recreate:
1. Enable SavedSearchFilters syspref
2. Perform search with bib-level:("a")
3. Click "Save search as filter" and save
4. Perform search containing scandic character (åöä) 
5. Use filter you just created
=> notice that scandic characters in search term displayed incorrectly e.g. ä turns to ä
Comment 1 Owen Leonard 2023-05-15 17:53:31 UTC
Where is the malformed text? It looks correct to me when it's shown in the sidebar in both the OPAC and the staff interface.
Comment 2 Katrin Fischer 2023-05-15 21:28:57 UTC
I am seeing the issue too.

* Activate search filters
* Search for the
* Save as search filter
* Search for ü
* Limit to saved filter

No results match your search for 'kw,wrdl: "ü"' with limit(s): 'and (the)'.
Comment 3 Ville Korjus 2023-05-23 07:24:33 UTC Comment hidden (obsolete)
Comment 4 Sam Lau 2023-05-24 20:03:52 UTC
Created attachment 151653 [details] [review]
Bug 33734: Use raw filters with search filter links in facets.inc

Using html filters breaks scandic characters in search term
if one uses custom search filters. Use raw filters instead.

To test:
1. Enable SavedSearchFilters syspref
2. Perform search with bib-level:("a")
3. Click "Save search as filter" and save
4. Perform search containing scandic character (åöä)
5. Use filter you just created
=> notice that scandic characters in search term are displayed
incorrectly e.g. ä turns to ä
6. Apply this patch
7. Repeat steps 4 and 5
=> scandic characters should now be displayed correctly

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Sam Lau <samalau@gmail.com>
Comment 5 Jonathan Druart 2023-05-25 12:52:33 UTC
This code is wrong, we should reuse what we are doing later in the same file (search 'local_url').
Note that we are missing count and sort_by.

It's not your patch that is wrong but the existing code.
Comment 6 Jonathan Druart 2023-05-25 12:53:14 UTC
I guess the OPAC side should be fixed as well.
Comment 7 Jonathan Druart 2023-07-05 10:35:57 UTC
Do you think you could have another look at this one?
Comment 8 Jonathan Druart 2023-07-05 10:36:17 UTC
*** Bug 34192 has been marked as a duplicate of this bug. ***
Comment 9 David Cook 2023-09-13 02:03:54 UTC
Definitely don't want to be using the "raw" filter there.
Comment 10 David Cook 2023-09-13 02:06:10 UTC
(In reply to Katrin Fischer from comment #2)
> I am seeing the issue too.
> 
> * Activate search filters
> * Search for the
> * Save as search filter
> * Search for ü
> * Limit to saved filter
> 
> No results match your search for 'kw,wrdl: "ü"' with limit(s): 'and (the)'.

Thanks for that plan. I confirm that it happens too.
Comment 11 David Cook 2023-09-13 02:22:45 UTC
(In reply to David Cook from comment #9)
> Definitely don't want to be using the "raw" filter there.

Actually, I'm wrong. We do want to be using the "raw" filter here, because the URL is already encoded.

However, Ville makes a couple of mistakes in their patch. 

First, we should escape search_filter.id for completeness even though it's system generated.

Second, we need to escape search_filter.name since that is user generated and could cause XSS if it's not HTML escaped.
Comment 12 David Cook 2023-09-13 02:27:28 UTC
Created attachment 155582 [details] [review]
Bug 33734: Remove incorrect URL escaping on search filters

This patch removes an incorrect URL escaping on search filter
facet URLs.

Without this patch, search filters using non-ASCII characters won't
return results.

To test:
0. Apply patch
1. Enable SavedSearchFilters syspref
2. Search for "the"
3. Save as search filter
4. Search for "ü"
5. Apply the "the" filter
6. Note that search results appear
Comment 13 PTFS Europe Sandboxes 2023-09-13 06:16:49 UTC
Created attachment 155583 [details] [review]
Bug 33734: Remove incorrect URL escaping on search filters

This patch removes an incorrect URL escaping on search filter
facet URLs.

Without this patch, search filters using non-ASCII characters won't
return results.

To test:
0. Apply patch
1. Enable SavedSearchFilters syspref
2. Search for "the"
3. Save as search filter
4. Search for "ü"
5. Apply the "the" filter
6. Note that search results appear

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 14 Michaela Sieber 2023-09-13 06:17:58 UTC
Great, thanks David :-)
Comment 15 David Cook 2023-09-13 06:44:22 UTC
(In reply to Michaela Sieber from comment #14)
> Great, thanks David :-)

Thanks, Michaela!

Although I just realized I forgot to fix the OPAC, so I need to mark it as Failed QA...
Comment 16 David Cook 2023-09-13 07:02:50 UTC
Created attachment 155584 [details] [review]
Bug 33734: Remove incorrect URL escaping on search filters

This patch removes an incorrect URL escaping on search filter
facet URLs.

Without this patch, search filters using non-ASCII characters won't
return results.

To test:
0. Apply patch
1. Enable SavedSearchFilters syspref
2. Search for "the"
3. Save as search filter (to show on both OPAC and staff interface)
4. Search for "ü" in staff interface
5. Apply the "the" filter
6. Note that search results appear
7. Search for "ü" in OPAC
8. Apply the "the" filter
9. Note that search results appear
Comment 17 David Cook 2023-09-13 07:03:09 UTC
(In reply to Michaela Sieber from comment #14)
> Great, thanks David :-)

If you want to test again now, that would be great!
Comment 18 PTFS Europe Sandboxes 2023-09-13 07:38:24 UTC
Created attachment 155585 [details] [review]
Bug 33734: Remove incorrect URL escaping on search filters

This patch removes an incorrect URL escaping on search filter
facet URLs.

Without this patch, search filters using non-ASCII characters won't
return results.

To test:
0. Apply patch
1. Enable SavedSearchFilters syspref
2. Search for "the"
3. Save as search filter (to show on both OPAC and staff interface)
4. Search for "ü" in staff interface
5. Apply the "the" filter
6. Note that search results appear
7. Search for "ü" in OPAC
8. Apply the "the" filter
9. Note that search results appear

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 19 Katrin Fischer 2023-09-15 14:48:59 UTC
Created attachment 155690 [details] [review]
Bug 33734: Remove incorrect URL escaping on search filters

This patch removes an incorrect URL escaping on search filter
facet URLs.

Without this patch, search filters using non-ASCII characters won't
return results.

To test:
0. Apply patch
1. Enable SavedSearchFilters syspref
2. Search for "the"
3. Save as search filter (to show on both OPAC and staff interface)
4. Search for "ü" in staff interface
5. Apply the "the" filter
6. Note that search results appear
7. Search for "ü" in OPAC
8. Apply the "the" filter
9. Note that search results appear

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 20 Tomás Cohen Arazi 2023-09-19 19:35:30 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 21 Fridolin Somers 2023-09-21 06:17:41 UTC
In my opinion we could have used a more simple syntaxe :
<a href="[% base_url | $raw %]&nolimit=search_filter:[% search_filter.id | uri %]">[x]</a>
Comment 22 Fridolin Somers 2023-09-21 06:18:27 UTC
Pushed to 23.05.x for 23.05.04
Comment 23 David Cook 2023-09-21 23:12:13 UTC
(In reply to Fridolin Somers from comment #21)
> In my opinion we could have used a more simple syntaxe :
> <a href="[% base_url | $raw %]&nolimit=search_filter:[% search_filter.id |
> uri %]">[x]</a>

Yep that's true. Looking at it again, that's obvious now. "I missed the forest for the trees" 😬.
Comment 24 Matt Blenkinsop 2023-10-12 16:11:05 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x