Bug 27581

Created attachment 116078 [details] [review]
Bug 27581: Rename USeICU preference to UseICUStyleQuotes

Rather thna have a poorly named preference with an explanation of how it doesn't
do what it implies it does, let's rename it!

To test:
 1 - Set UseICU to 'Using'
 2 - Go to the details page of a record in the staff interface
 3 - Hover over a subject heading that has subfields
 4 - Note the link has curly brackets around the subject, like:
     http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=su:{Winnie-the-Pooh Fictitious character}
 5 - Change UseICU to 'Not using'
 6 - Note the link is now:
     http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=su:"Winnie-the-Pooh Fictitious character"
 7 - Repeat on OPAC
 8 - Apply patch
 9 - Restart all
10 - Repeat tests, the behaviour has not changed
11 - Read the new syspref description and confirm it makes sense

Created attachment 116082 [details] [review]
Bug 27581: Rename USeICU preference to UseICUStyleQuotes

Rather thna have a poorly named preference with an explanation of how it doesn't
do what it implies it does, let's rename it!

To test:
 1 - Set UseICU to 'Using'
 2 - Go to the details page of a record in the staff interface
 3 - Hover over a subject heading that has subfields
 4 - Note the link has curly brackets around the subject, like:
     http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=su:{Winnie-the-Pooh Fictitious character}
 5 - Change UseICU to 'Not using'
 6 - Note the link is now:
     http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=su:"Winnie-the-Pooh Fictitious character"
 7 - Repeat on OPAC
 8 - Apply patch
 9 - Restart all
10 - Repeat tests, the behaviour has not changed
11 - Read the new syspref description and confirm it makes sense

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Hi Nick, I am sorry, but some unrelated changes slipped into this patch:
 koha-tmpl/intranet-tmpl/prog/en/modules/tags/review.tt

Created attachment 116154 [details] [review]
Bug 27581: Rename USeICU preference to UseICUStyleQuotes

Rather thna have a poorly named preference with an explanation of how it doesn't
do what it implies it does, let's rename it!

To test:
 1 - Set UseICU to 'Using'
 2 - Go to the details page of a record in the staff interface
 3 - Hover over a subject heading that has subfields
 4 - Note the link has curly brackets around the subject, like:
     http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=su:{Winnie-the-Pooh Fictitious character}
 5 - Change UseICU to 'Not using'
 6 - Note the link is now:
     http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=su:"Winnie-the-Pooh Fictitious character"
 7 - Repeat on OPAC
 8 - Apply patch
 9 - Restart all
10 - Repeat tests, the behaviour has not changed
11 - Read the new syspref description and confirm it makes sense

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

(In reply to Katrin Fischer from comment #3)
> Hi Nick, I am sorry, but some unrelated changes slipped into this patch:
>  koha-tmpl/intranet-tmpl/prog/en/modules/tags/review.tt

Oops, reverted those

Created attachment 116455 [details] [review]
Bug 27581: Rename USeICU preference to UseICUStyleQuotes

Rather thna have a poorly named preference with an explanation of how it doesn't
do what it implies it does, let's rename it!

To test:
 1 - Set UseICU to 'Using'
 2 - Go to the details page of a record in the staff interface
 3 - Hover over a subject heading that has subfields
 4 - Note the link has curly brackets around the subject, like:
     http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=su:{Winnie-the-Pooh Fictitious character}
 5 - Change UseICU to 'Not using'
 6 - Note the link is now:
     http://localhost:8081/cgi-bin/koha/catalogue/search.pl?q=su:"Winnie-the-Pooh Fictitious character"
 7 - Repeat on OPAC
 8 - Apply patch
 9 - Restart all
10 - Repeat tests, the behaviour has not changed
11 - Read the new syspref description and confirm it makes sense

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Created attachment 116456 [details] [review]
Bug 27581: (QA follow-up) Fix occurence in it-IT custom.sql

Created attachment 116498 [details] [review]
Bug 27581: Fix missing occurrences in tests

Pushed to master for 21.05, thanks to everybody involved!

Sorry, not sure how the test ones escaped me. hmpf.

I prefer not change syspref on stable branch.
This may be used in home-made XSLT files.

Not pushed to 20.11.x

Just putting here what the manual said about this pref.

ICU is a set of code libraries providing Unicode and Globalization support for software applications. What this means is ICU Zebra indexing is only necessary if you use non-roman characters in your cataloging. If using ICU Zebra indexing you will want to not use QueryFuzzy.

This setting will not affect Zebra indexing, it should only be used
to tell Koha that you have activated ICU indexing if you have
actually done so, since there is no way for Koha to figure this out
on its own.

Does the 'new' system preference still block QueryFuzzy from working? Do the different quotes (either double quotes or curly brackets) have an influence when tracing subjects using non-roman characters?

(In reply to Caroline Cyr La Rose from comment #12)
> Just putting here what the manual said about this pref.
> 
> ICU is a set of code libraries providing Unicode and Globalization support
> for software applications. What this means is ICU Zebra indexing is only
> necessary if you use non-roman characters in your cataloging. If using ICU
> Zebra indexing you will want to not use QueryFuzzy.
> 
> This setting will not affect Zebra indexing, it should only be used
> to tell Koha that you have activated ICU indexing if you have
> actually done so, since there is no way for Koha to figure this out
> on its own.
> 
> Does the 'new' system preference still block QueryFuzzy from working? Do the
> different quotes (either double quotes or curly brackets) have an influence
> when tracing subjects using non-roman characters?

There are several places on the wiki that cover using ICU chains - it is much more involved than a syspref swtich (well, it's a switch in two config files)
https://wiki.koha-community.org/wiki/ICU_chains_configuration
https://wiki.koha-community.org/wiki/ICU_do_not_undiacritic
https://wiki.koha-community.org/wiki/Correcting_Search_of_Arabic_records

ICU still doesn't work with queryfuzzy afaik

I did not find the documentation from Zebra on the use of quotes, but I assume ICU requires the change - many of our partner sites were using non-ICU indexing with ICU style quotes, so at least one=-way it is not harmful

(In reply to Nick Clemens from comment #13)
> I did not find the documentation from Zebra on the use of quotes, but I
> assume ICU requires the change - many of our partner sites were using
> non-ICU indexing with ICU style quotes, so at least one=-way it is not
> harmful

Actually, now that I think about this again, I think that Koha's idea of "ICU quotes" is totally wrong.

Koha doesn't speak ICU or CHR to Zebra. Koha just sends PQF queries to Zebra (and it uses a CCL2RPN conversion to produce these PQF queries). The quotes are relevant only in terms of CCL and PQF queries. So... they're relevant regardless of indexing mode (which basically just governs normalization and tokenization).

Now... Bug 27299 shows and solves a tokenization issue with ICU and phrase registers, and that caused there to be a difference in how records were being indexed. 

I see that there is a TraceCompleteSubfields system preference which adds a qualifier of ",complete-subfield", which would cause the phrase register to be used as it's a 6=2 attribute (see https://github.com/indexdata/idzebra/blob/master/tab/default.idx for details on the register)

Technically speaking, we should always be wrapping the operands in double quotes so that queries are like this: au="Cook, David" and su="ICU".

Oh I see... with UseICU, we're using curly braces instead of double quotes. Yeah... that's not an ICU thing. 

You can see some discussion about this here:
http://lists.indexdata.dk/pipermail/zebralist/2007-July/001674.html
http://lists.indexdata.dk/pipermail/yazlist/2001-September/000090.html

I think it might be a case of poor string handling in Perl in the past. Going to dredge up some more detail..

I think that curly braces are just an undocumented synonym for double quotes. We could reach out to Indexdata for comment on that.

This is using ICU:

Z> find su,complete-subfield={Rock musicians United States Biography 1705}
Sent searchRequest.
Received SearchResponse.
Search was a success.
Number of hits: 2, setno 16
SearchResult-1: term=Rock musicians United States Biography 1705 cnt=2
records returned: 0
Elapsed: 0.000290

Z> find su,complete-subfield="Rock musicians United States Biography 1705"
Sent searchRequest.
Received SearchResponse.
Search was a success.
Number of hits: 2, setno 17
SearchResult-1: term=Rock musicians United States Biography 1705 cnt=2
records returned: 0
Elapsed: 0.000361

So UseICUStyleQuotes doesn't really make sense to me. It's probably just hiding a bug in Koha.

Note that the above used the fix from Bug 27299, but I just tried again without the tokenization fix, and both {} and "" works fine.

Looking at Bug 7092 which introduced UseICU... it looks like this was always a workaround. 

If someone has a test plan that shows ICU not working with "" or CHR not working with {}, I'd happily test it.