Closed Bug 386434 Opened 16 years ago Closed 15 years ago

Add-ons aren't sorted correctly in Add-ons manager (problem with non-ascii chars)

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla1.9.1a2

People

(Reporter: JasnaPaka, Assigned: regis.caspar+bz)

Details

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (Windows; U; Win98; cs; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Win98; cs; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4

Tested with Firefox 2.0.0.4 but same problem with latest Firefox Trunk.

Reproducible: Always

Steps to Reproduce:
1. Install Czech dictionary from https://addons.mozilla.org/en-US/firefox/browse/type:3.

Actual Results:  
In Add-ons manager have add-ons this sequence:
* DOM Inspector
* Talkback
* České slovníky pro kontrolu pravopisu


Expected Results:  
Right sequence is:
* České slovníky pro kontrolu pravopisu
* DOM Inspector
* Talkback
Product: Firefox → Toolkit
I think so, too.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Should be a fairly simple fix replacing the alphabetic comparison with something more locale aware.
Whiteboard: [good first bug]
Attached patch patch proposalSplinter Review
Here's a patch proposal using String.localCompare (will attach screenshot showing results)
Attachment #333913 - Flags: review?(dtownsend)
Attached image screenshot (results)
Status: NEW → ASSIGNED
Comment on attachment 333913 [details] [diff] [review]
patch proposal

Excellent, thanks. Just a quick tip for the future, it helps to provide more context with the patches, 8 lines is normally recommended.

Say if you are unable to check in.
Attachment #333913 - Flags: review?(dtownsend) → review+
Suggestion: remove the 'toLowerCase' calls.  They are no longer needed because localeCompare sorts 'a' next to 'A' (unlike '<' and '>' which compare unicode codes).  For example:

old '>' and '<' sorts by Unicode code:

["a", "b", "c", "d","A", "B", "C", "D", "Č"].sort(function compare(a,b) a<b? -1 : a > b? 1 : 0)
--> A,B,C,D,a,b,c,d,Č

new localeCompare sorts upper and lowercase adjacent:

["a", "b", "c", "d","A", "B", "C", "D", "Č"].sort(String.localeCompare)
--> a,A,b,B,c,C,Č,d,D


Attached patch patch v2Splinter Review
Same patch as before (with context) and without toLowerCase() calls. I also changed the comment above the compare function to "Locale sensitive sort".

Tested with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a2pre) Gecko/20080814225749 Minefield/3.1a2pre ID:20080814225749 (custom build)
Assignee: nobody → regis.caspar+bz
Comment on attachment 334070 [details] [diff] [review]
patch v2

Nice, thanks
Attachment #334070 - Flags: review+
Keywords: checkin-needed
Landed: http://hg.mozilla.org/index.cgi/mozilla-central/rev/67d16a172040
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [good first bug]
Target Milestone: --- → mozilla1.9.1a2
You need to log in before you can comment on or make changes to this bug.