Bug 117353 – window.find() case-insensitive search doesn't match diacritical marks

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Daniel Bates]

Consider a web page that contains the spanish word "que". Then window.find("qué", false /* Case sensitive */) returns true. That is, we found a match since we consider the word "qué" to be equivalent to the word "que". Instead we should consider the accented e as being more specific and hence window.find() should return false (i.e. not found).

[Daniel Bates]

<rdar://problem/13510672>

[Daniel Bates]

Created attachment 204058 [details]
Patch

[Daniel Bates]

(In reply to comment #2)
> Created an attachment (id=204058) [details]
> Patch

As a side effect of this change our case-insensitive search behavior for window.find()
will more closely match the same behavior in Firefox.

[Daniel Bates]

Created attachment 204062 [details]
Patch

Remove tab characters that I inadvertently added to file LayoutTests/fast/text/test-find.js. Also, write testFind() in terms of variable CaseInsensitive.

[Darin Adler]

Comment on attachment 204058 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204058&action=review

> LayoutTests/fast/text/find-case-folding.html:40
> -            if (!canFind("Stra" + smallSharpS + "e", "Strasse")) {
> +            if (canFind("Stra" + smallSharpS + "e", "Strasse")) {
>                  success = false;
> -                message += " Cannot find ss when searching for small sharp s.";
> +                message += " Found ss when searching for small sharp s.";
>              }

Is this change an improvement? I think it was good that searching for ß found ss.

[Alexey Proskuryakov]

I asked Dan on IRC to review some of older bugs we had about diacritics (in)sensitive search, I think that user expectations may be more nuanced.

[Daniel Bates]

(In reply to comment #6)
> I asked Dan on IRC to review some of older bugs we had about diacritics (in)sensitive search, I think that user expectations may be more nuanced.

From looking at the related bugs, the proposed change will make our search behavior match people's expectations when searching for accented characters.

Moreover, it turned out that the Radar bug for this issue, <rdar://problem/13510672>, is a duplicate of <rdar://problem/8535227>.

[Daniel Bates]

(In reply to comment #5)
> (From update of attachment 204058 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204058&action=review
> 
> > LayoutTests/fast/text/find-case-folding.html:40
> > -            if (!canFind("Stra" + smallSharpS + "e", "Strasse")) {
> > +            if (canFind("Stra" + smallSharpS + "e", "Strasse")) {
> >                  success = false;
> > -                message += " Cannot find ss when searching for small sharp s.";
> > +                message += " Found ss when searching for small sharp s.";
> >              }
> 
> Is this change an improvement? I think it was good that searching for ß found ss.

Disregarding the case with ß for the moment (I talk about it below), the proposed patch is an improvement because it makes the behavior of window.find() more closely match a person's expectation when searching for special characters based on our in-person discussion this morning (06/07). This behavior change has also been requested by various people as seen in the bug reports related to <rdar://problem/8535227>.

Should we look to special case our handling of ß? ICU regards ß similar to an accented character (é) when using strength UCOL_SECONDARY.

[Daniel Bates]

(In reply to comment #8)
> (In reply to comment #5)
> > (From update of attachment 204058 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=204058&action=review
> > 
> > > LayoutTests/fast/text/find-case-folding.html:40
> > > -            if (!canFind("Stra" + smallSharpS + "e", "Strasse")) {
> > > +            if (canFind("Stra" + smallSharpS + "e", "Strasse")) {
> > >                  success = false;
> > > -                message += " Cannot find ss when searching for small sharp s.";
> > > +                message += " Found ss when searching for small sharp s.";
> > >              }
> > 
> [...]
> Should we look to special case our handling of ß? ICU regards ß similar to an accented character (é) when using strength UCOL_SECONDARY.

From talking with Darin today (06/11) in person,  we should look to map ß to "ss". For now, we should land expected failure results for test LayoutTests/fast/text/find-case-folding.html instead of updating it to pass.

[Daniel Bates]

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #5)
> > > (From update of attachment 204058 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=204058&action=review
> > > 
> > > > LayoutTests/fast/text/find-case-folding.html:40
> > > > -            if (!canFind("Stra" + smallSharpS + "e", "Strasse")) {
> > > > +            if (canFind("Stra" + smallSharpS + "e", "Strasse")) {
> > > >                  success = false;
> > > > -                message += " Cannot find ss when searching for small sharp s.";
> > > > +                message += " Found ss when searching for small sharp s.";
> > > >              }
> > > 
> > [...]
> > Should we look to special case our handling of ß? ICU regards ß similar to an accented character (é) when using strength UCOL_SECONDARY.
> 
> From talking with Darin today (06/11) in person,  we should look to map ß to "ss". For now, we should land expected failure results for test LayoutTests/fast/text/find-case-folding.html instead of updating it to pass.

Filed bug #117548 to map ß to "ss".

[Daniel Bates]

Committed r151505: <http://trac.webkit.org/changeset/151505>

[Allan Sandfeld Jensen]

This patch raised the minimum required libicu to 4.4, was that intentional?