newly added languages should appear on top of the list

RESOLVED FIXED in Firefox 11

Status

()

Firefox
Preferences
--
enhancement
RESOLVED FIXED
13 years ago
6 years ago

People

(Reporter: Steffen Wilberg, Assigned: Javi Rueda)

Tracking

unspecified
Firefox 11
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

13 years ago
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".
(Reporter)

Comment 1

13 years ago
*** Bug 252194 has been marked as a duplicate of this bug. ***
Keywords: helpwanted
I'm assuming by the fact that Steffen filed this twice that he intends to do
this himself.  :)

Steffen, sooner is better on this :)
Keywords: helpwanted
sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change
QA Contact: mconnor → preferences
(Assignee)

Comment 4

6 years ago
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).
Assignee: steffen.wilberg → leofigueres
Attachment #574082 - Flags: review?(vladimir)
(Reporter)

Comment 5

6 years ago
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
(Assignee)

Comment 6

6 years ago
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 :-)
(Reporter)

Comment 7

6 years ago
It probably doesn't make a big difference - as long as you don't pick Gavin :) And this bug is definitely yours.
(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 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(",");
Attachment #574082 - Flags: review?(vladimir) → review-
(Reporter)

Comment 10

6 years ago
(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 :)
(Assignee)

Comment 11

6 years ago
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.
Attachment #574082 - Attachment is obsolete: true
Attachment #577457 - Flags: review?(gavin.sharp)
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.
(Reporter)

Comment 13

6 years ago
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.
Attachment #577457 - Flags: review?(gavin.sharp) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
(Reporter)

Comment 14

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/0f4d49ced402
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0f4d49ced402
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
You need to log in before you can comment on or make changes to this bug.