294050 - Search in history is case-sensitive for non-ascii letters

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Daniel Ovsiannikov]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

Search in history do distinguish between uppercase and lowercase non-english
letters. This bug make difficulties for searching words and search will produce
non-complete results for users not knowing about this bug. 

Reproducible: Always

Steps to Reproduce:

Comment 1

[u60234]

Confirming bug with trunk build from 20050513 on Win XP.

Comment 2

[Daniel Ovsiannikov]

Can I request a blocker?

Comment 3

[djet]

Will this bug be fixed or the new Fx *again* will be bundled with *one more* 
broken feature? 

Comment 4

[Jesse Ruderman]

Daniel, can you add specific steps to reproduce?  (What language is involved,
what pages to visit, what to search for.)

Comment 5

[u60234]

I'm not Daniel, but...

Steps to reproduce:
1. Visit http://www.fyndborsen.se/
2. Open History and type in "fyndb" in the search field.
3. Press Alt+0246 to create a "ö" (latin small letter o with diaeresis)
4. Notice how the entry "FYNDBÖRSEN - Hemelektronik på internet sedan 1997" does
not match.
5. Delete the "ö" and press Alt+0214 to create "Ö" (latin capital letter o with
diaeresis) and notice the match.

Comment 6

[Jesse Ruderman]

Thanks.  I can reproduce the bug using Mozilla/5.0 (Macintosh; U; PPC Mac OS X
Mach-O; en-US; rv:1.9a1) Gecko/20051006 Firefox/1.6a1.

Comment 7

[Jesse Ruderman]

nsGlobalHistory::RowMatches uses nsCaseInsensitiveCStringComparator, which is
for comparing ASCII strings.  According to bug 231782, implementing
nsCaseInsensitiveUTF8StringComparator would not be trivial.

I will try to fix this bug by using UTF-16 or UCS2 instead of UTF-8 in
nsGlobalHistory::RowMatches, since nsCaseInsensitiveStringComparator takes
UTF-16 or UCS2.

Comment 8

[Daniel Ovsiannikov]

But there is a working search field in the Bookmarks sidebar. Wouldn't it be 
easy just to "copy" its behaviour? 

Comment 9

[Jesse Ruderman]

Attachment: patch

Comment 10

[Jesse Ruderman]

Attachment: patch

Comment 11

[Jo Hermans]

*** Bug 311660 has been marked as a duplicate of this bug. ***

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 198917 [details] [diff] [review]
patch

>+      const nsString searchText = term->text;

Make that |const nsString& searchText| perhaps?

sr=bzbarsky with that.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Neil, Ctho, does the other global history need a similar change?

Comment 14

[Jesse Ruderman]

> >+      const nsString searchText = term->text;

> Make that |const nsString& searchText| perhaps?

term->text is an nsXPIDLString, so I think |const nsXPIDLString& searchText|
would be more clear.  |const nsString& searchText| does work (and avoid copying
the string buffer) but relies on the fact that nsXPIDLString is a subclass of
nsString.

Comment 15

[Jesse Ruderman]

Attachment: patch

This patch declares searchText as |const nsXPIDLString&|.

Comment 16

[[e-mail]]

(In reply to comment #13)
>Neil, Ctho, does the other global history need a similar change?
Almost certainly, which is why I created bug 311660...

Comment 17

[Jesse Ruderman]

Checked in on trunk.

Comment 18

[Daniel Ovsiannikov]

On trunk?? Does that mean it won't be fixed for 1.5?? >:-E 

Comment 19

[Jesse Ruderman]

Verified:

* [Mac, Win] The fix works as advertised in today's trunk nightlies using normal
History sidebar search.

* [Mac] All History sidebar modes still work (except for "By Site", which has
been broken for ages, see bug 206927).

* [Win] The fix works as advertised using the "Enhanced History Manager" extension.

* [Win] The "Enhanced History Manager" extension still works.  I tested all
search mode combinations ("title starts with", etc) and all grouping options. 
The only oddity I found was that "location is not
http://steelgryphon.com/blog/?p=62" caused the remaining items to be re-sorted,
but that happens in a trunk build from last week too.

* [Mac] The "How'd I Get Here" extension still works.

Comment 20

[Jesse Ruderman]

I think this bug makes history search pretty broken in some languages, so I'm
requesting approval for Firefox 1.5 RC 1.

Comment 21

[Scott MacGregor]

plussing since we approved this patch.

Comment 22

[Jesse Ruderman]

Checked in on MOZILLA_1_8_BRANCH.