Last Comment Bug 246827 - newly added languages should appear on top of the list
: newly added languages should appear on top of the list
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Preferences (show other bugs)
: unspecified
: All All
: -- enhancement with 2 votes (vote)
: Firefox 11
Assigned To: Javi Rueda
:
:
Mentors:
: 252194 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-06-15 06:46 PDT by Steffen Wilberg
Modified: 2011-12-05 10:31 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add the new language at the top of the list (1.57 KB, patch)
2011-11-12 12:35 PST, Javi Rueda
gavin.sharp: review-
Details | Diff | Splinter Review
patch 1.1 (1.51 KB, patch)
2011-11-28 18:32 PST, Javi Rueda
gavin.sharp: review+
Details | Diff | Splinter Review

Description Steffen Wilberg 2004-06-15 06:46:58 PDT
In Tools->Options->General->Languages, if you select a language to add, and
click the "Add" button, the language appears at the bottom of the list of active
languages. It should appear on top of the list instead.

Bug 181541 comment 110 said:
> I expect that new users want to "Add" a language because they want their
> favorite web site to show up in their language if it's available.
> So I would like that adding a language adds on top of the list, first position,
> instead of last position. The basic configuration steps would be to add my
> native language, then bump OK, that's all, don't need to "Move up".
Comment 1 Steffen Wilberg 2004-07-19 16:52:05 PDT
*** Bug 252194 has been marked as a duplicate of this bug. ***
Comment 2 Mike Connor [:mconnor] 2004-07-19 18:57:45 PDT
I'm assuming by the fact that Steffen filed this twice that he intends to do
this himself.  :)

Steffen, sooner is better on this :)
Comment 3 Mike Connor [:mconnor] 2006-08-27 06:58:57 PDT
sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change
Comment 4 Javi Rueda 2011-11-12 12:35:33 PST
Created attachment 574082 [details] [diff] [review]
Add the new language at the top of the list

Adds the new language at the top of the array (at the function), concatenates the old string and then makes it the new preference value.

Requesting review to :vlad as the file being modified by the patch hasn't been reviewed by anybody (as per the log info).

During my testing, I've noticed that, when opening the Languages dialog box, the list of available languages includes those that have been accepted by user, also. When adding a new one, this list is updated. Don't know if this is a desired behaviour (I doubt it). This is also how it works on Firefox 8 Windows (last release at the date this is written).
Comment 5 Steffen Wilberg 2011-11-16 16:52:54 PST
Thanks for taking this, I forgot about his bug years ago...

(In reply to Javi Rueda from comment #4)
> Requesting review to :vlad as the file being modified by the patch hasn't
> been reviewed by anybody (as per the log info).
I reintroduced the dialog in bug 181541 (r=mconnor).
The code got refactored with the introduction of <preference> element by Ben Goodger (bug 274712, no review; he also introduced the trailing whitespace and the typo...).
Further changes are in the CVS log:
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/browser/components/preferences/languages.js&rev=HEAD&mark=1.10
Comment 6 Javi Rueda 2011-11-16 17:32:34 PST
Is Mano a better reviewer of this code? Also, if you are intereted again in this bug, you can take it again, no problem to me :-)
Comment 7 Steffen Wilberg 2011-11-17 08:48:31 PST
It probably doesn't make a big difference - as long as you don't pick Gavin :) And this bug is definitely yours.
Comment 8 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-11-22 12:41:44 PST
(In reply to Steffen Wilberg from comment #7)
> It probably doesn't make a big difference - as long as you don't pick Gavin

Please stop spreading the rumour that I'm not responsive to review requests, since it isn't true :)
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-11-22 12:47:18 PST
Comment on attachment 574082 [details] [diff] [review]
Add the new language at the top of the list

Just use:
if (preference.value == "")
  preference.value = selectedID;
else
  preference.value = arrayOfPrefs.unshift(selectedID).join(",");
Comment 10 Steffen Wilberg 2011-11-22 13:11:26 PST
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #8)
> Please stop spreading the rumour that I'm not responsive to review requests,
> since it isn't true :)
Guess I should do a better job at nagging about 8-week-old review requests then :)
Comment 11 Javi Rueda 2011-11-28 18:32:47 PST
Created attachment 577457 [details] [diff] [review]
patch 1.1

Using code from comment 9.

unshift() returns an integer, not the array object, so code had to be split up into unshifting and joining.
Comment 12 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-11-30 12:33:21 PST
Comment on attachment 577457 [details] [diff] [review]
patch 1.1

>diff --git a/browser/components/preferences/languages.js b/browser/components/preferences/languages.js

>     var arrayOfPrefs = preference.value.toLowerCase().split(/\s*,\s*/);

Using arrayOfPrefs to set the value will lower-case the pref, is that a problem? Looks like it isn't, but would be good to confirm.
Comment 13 Steffen Wilberg 2011-11-30 12:53:19 PST
http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/languages.js#66

resource://gre/res/language.properties is the full list, including languages we don't show in the UI, and it's all lower-case, language and region.

According to http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.10, all tags are case- insensitive.
Comment 15 Matt Brubeck (:mbrubeck) 2011-12-05 10:31:54 PST
https://hg.mozilla.org/mozilla-central/rev/0f4d49ced402

Note You need to log in before you can comment on or make changes to this bug.