Summary: | window.find() case-insensitive search doesn't match diacritical marks | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||||
Component: | HTML Editing | Assignee: | Daniel Bates <dbates> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | allan.jensen, ap, darin, rniwa | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Daniel Bates
2013-06-07 10:33:53 PDT
Created attachment 204058 [details]
Patch
(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. 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.
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. 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. (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>. (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. (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. (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". Committed r151505: <http://trac.webkit.org/changeset/151505> This patch raised the minimum required libicu to 4.4, was that intentional? |