Summary: | System preferences search does not allow accented letters | ||
---|---|---|---|
Product: | Koha | Reporter: | Fridolin Somers <fridolin.somers> |
Component: | System Administration | Assignee: | Fridolin Somers <fridolin.somers> |
Status: | CLOSED FIXED | QA Contact: | Mason James <mtj> |
Severity: | normal | ||
Priority: | P5 - low | CC: | chris, gmcharlt, matted-34813, mtj, paul.poulain, ruth, stephane.delaye |
Version: | 3.8 | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Proposed patch
Proposed patch (revised) Proposed patch (rebased master) SIGNED-OFF-Bug-8702-System-preferences-search-accented-sysprefs |
Description
Fridolin Somers
2012-08-29 14:49:15 UTC
Created attachment 11879 [details] [review] Proposed patch Very small patch. Replaces strong regexp replacement by just string trim. Hmm this is quite a big change, it now allows you to search on anything, only strip trailing space (and combining multiple whitespace into 1) While I don't think this is a problem we have gone from a whitelist to a blacklist I don't know if there are any characters you could use to make a grep behave badly, but i'd like someone else to tell me I'm being paranoid before I sign off :) It might be better to use this: $searchfield =~ s/\p{IsC}//g; #Toss out control codes, Unicode compliant Fridolyn's patch would toss *some* non-word characters--spaces, tabs, newlines, and formfeeds--when they appear at the ends of the string, but would not deal with escape characters or other undesirable inputs. Using this statement would express the original intent in a fully-Unicode-compliant way. Suggest changing the patch to the example above. It will pass letters, marks, numbers, punctuation, symbols, and separators. Chris, your paranoia is a good thing! Any of the other classes of character probably are okay--although most won't match anything--but the control codes are probably a good thing to toss out. (Note: I haven't tried this myself in the Koha context; I've used it elsewhere. Definitely, someone wants to TEST THIS. It's pretty arcane.) Created attachment 11923 [details] [review] Proposed patch (revised) New patch with previous comment proposition. This patch : - removes tailing and leading spaces in search term. - replaces several spaces by one. - removes Unicode control characters. - adds some missing HTML filters for search term in TT. You can check with %9F in URL. See http://www.regular-expressions.info/unicode.html I tried to apply the patch. I got: Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/stopwords-admin-search.inc CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/stopwords-admin-search.inc I don't even see stopwords-admin-search.inc mentioned in your patch. So I am too confused to make any assumptions. See: http://paste.koha-community.org/173 Can you rebase and resubmit? Created attachment 12311 [details] [review] Proposed patch (rebased master) Rebased patch. Previous patch applying failed because one of the files it modified is deleted now. Created attachment 12324 [details] [review] SIGNED-OFF-Bug-8702-System-preferences-search-accented-sysprefs So i wouldn't need to switch to another language, I tested this way: 1) In my kohadev environment I changed: ./koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/tools.pref to have Jürgen at the end of displayed text for the ExportWithCsvProfile syspref. Tools: - - pref: ExportWithCsvProfile - CSV profile for export Jürgen 2) I used the admin syspref search, specifying "Jürgen" Result: not found. Plus I got an info message saying not found with "Jrgen" 3) applied patch 4) did the same search, and now I found the syspref with Jürgen. 5) I also searched for something I should not find üxxx and I got an info message which showed the ü Great work! wajasu Note: I took a stray "2" typo out of the original patch. Now folks with accents/diacritics in syspref text will match. (In reply to comment #7) > Created attachment 12324 [details] [review] > SIGNED-OFF-Bug-8702-System-preferences-search-accented-sysprefs patch looks good, passing QA… $ koha-qa.pl -c 1 testing 1 commit(s) (applied to commit bd1f621) * 30c1031 Bug 8702: System preferences search does not allow accented letters admin/preferences.pl koha-tmpl/intranet-tmpl/prog/en/includes/prefs-admin-search.inc koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences.tt * admin/preferences.pl OK * koha-tmpl/intranet-tmpl/prog/en/includes/prefs-admin-search.inc OK * koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences.tt OK Patch pushed to master Pushed to 3.8.x, will be in 3.8.6 *** Bug 8416 has been marked as a duplicate of this bug. *** |