Bug 8702 - System preferences search does not allow accented letters
Summary: System preferences search does not allow accented letters
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin SOMERS
QA Contact: Mason James
URL:
Keywords:
: 8416 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-08-29 14:49 UTC by Fridolin SOMERS
Modified: 2014-05-26 21:04 UTC (History)
7 users (show)

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


Attachments
Proposed patch (765 bytes, patch)
2012-08-29 14:52 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Proposed patch (revised) (3.48 KB, patch)
2012-08-31 16:04 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Proposed patch (rebased master) (2.62 KB, patch)
2012-09-18 14:04 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
SIGNED-OFF-Bug-8702-System-preferences-search-accented-sysprefs (2.81 KB, patch)
2012-09-19 04:33 UTC, wajasu
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin SOMERS 2012-08-29 14:49:15 UTC
System preferences search is not performed using .pref files which are translatable.
But actually, special letters are removed from search therm.
Comment 1 Fridolin SOMERS 2012-08-29 14:52:23 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2012-08-29 23:24:34 UTC
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 :)
Comment 3 D Ruth Bavousett 2012-08-31 12:47:29 UTC
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.)
Comment 4 Fridolin SOMERS 2012-08-31 16:04:28 UTC Comment hidden (obsolete)
Comment 5 wajasu 2012-09-14 16:15:48 UTC
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?
Comment 6 Fridolin SOMERS 2012-09-18 14:04:08 UTC Comment hidden (obsolete)
Comment 7 wajasu 2012-09-19 04:33:15 UTC
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.
Comment 8 Mason James 2012-09-25 11:17:09 UTC
(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
Comment 9 Paul Poulain 2012-09-26 17:09:05 UTC
Patch pushed to master
Comment 10 Chris Cormack 2012-09-28 21:02:46 UTC
Pushed to 3.8.x, will be in 3.8.6
Comment 11 Fridolin SOMERS 2013-02-15 15:51:23 UTC
*** Bug 8416 has been marked as a duplicate of this bug. ***