98.40 KB, image/png
1.97 KB, patch
|Details | Diff | Splinter Review|
132.54 KB, image/jpeg
OS: Android 4.4 Galaxy Nexus 4 Steps to reproduce: 1. Go to Settings -> Language -> Browser Language 2. Tap System default Actual Results: - the list has entries staring is both lower case and upper case making it to look really strange Expected Result: - the list entries should be all cosnistant
"Consistent" depends on your locale. If you're a Spanish speaker, it's weird that "English" has a capital letter. If you speak English, it's weird that "español" isn't capitalized. We could revert back to the previous behavior I implemented — to display every locale in the selected locale ("English", "Spanish" / "inglés", "español"), or we could do it this way. Doing it this way attempts to make everyone feel an island of comfort -- their own locale -- in a sea of discomfort (RTL, idiographic, accented language names). We could attempt to somehow sentence-case the string -- that's what HTC's locale picker seems to do -- but I actually think that's wrong, and I don't see any reliable mechanisms for doing so.
I believe we definitely need to show the localized label, not the English one. If the compromise for that is to show a mixed capitalization, I guess we need to adapt, even if it looks not great.
Android chooses to capitalize all the Latin named languages.
I dug into the Android source, and they do something horrrrrrrrible. http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/4.4.2_r1/com/android/internal/app/LocalePicker.java#219 They just forcibly uppercase the first letter. They don't even do a locale-aware uppercasing. Urgh. We can do it...
And then there's stuff like isiXhosa :-) I think the data is coming from CLDR, via ICU. I think that local names should follow local custom. Though, looking at what we do in languageNames.properties in gecko, we seem to disagree with CLDR. http://transvision.mozfr.org/string/?entity=toolkit/chrome/global/languageNames.properties:cs&repo=aurora for example.
Part of the complication is that there are (at least) two different representations of a noun like this. Spanish speakers would capitalize español at the start of a sentence, but not part-way through. Java's APIs don't make that distinction (for sadly predictable reasons), hence the awful hack in the Android source.
Axel, Flod: do you think that a solution identical to Android's -- call Java's toUpperCase on the first letter of the string -- is an acceptable choice here? The alternative is to define 40-odd new non-localized strings, and remember to add new ones as we add new locales. I don't mind doing that if it'll produce a better experience. Other suggestions welcome.
I'll leave the NI open for Pike, since I'm all but sure about this. I think capitalization is a problem when it comes to the middle of the sentence, while it should be more common to capitalize the beginning of a sentence. Example from my locale: language names are lowercase, but in this case it makes sense to have "Italiano", since it's like a stand-alone sentence. IMO the toUpperCase solution is better than the current mix of cases.
Assuming that we decide that capitalizing the first character is acceptable, here's a patch. Still building, so not yet tested.
Attachment #8437408 - Flags: review?(michael.l.comella)
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Patch tested and works.
Comment on attachment 8437408 [details] [diff] [review] Capitalize first letter of entries in locale switching UI. v1 Review of attachment 8437408 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/preferences/LocaleListPreference.java @@ +54,5 @@ > + } > + > + // For now, uppercase the first character of LTR locale names. > + // This is a reasonable hack for Bug 1014602, but it won't generalize > + // to all locales. optional nit: I'd add in that this is (roughly) what Android does.
Attachment #8437408 - Flags: review?(michael.l.comella) → review+
I'm torn on this one. There's a few that are border-line, but there are also languages for which a noun clearly goes like isiZulu or isiXhosa. Our language picker at the bottom of http://www.mozilla.org/ exposes both upper and lower case, and mixed-case, for example. That one's feeding from http://viewvc.svn.mozilla.org/vc/libs/product-details/json/languages.json?revision=123587&view=co, fwiw.
The approach Axel and I decided to take: we'll land this, and then have a follow-up to add an exceptions mechanism. That gives us a hook for Zulu and friends, and also provides an extension point for fake developer languages should we need them.
Comment on attachment 8437408 [details] [diff] [review] Capitalize first letter of entries in locale switching UI. v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): Cosmetic fix for Bug 917480. User impact if declined: Some languages will appear with a lowercase initial letter when uppercase is idiomatic. Testing completed (on m-c, etc.): Tested locally. Just landed on fx-team. Will evaluate on m-c prior to uplift. Risk to taking this patch (and alternatives if risky): Some languages might appear with a capital when they shouldn't. Minimal coding risk. String or IDL/UUID changes made by this patch: None.
Attachment #8437408 - Flags: approval-mozilla-aurora?
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Verified as fixed on Nightly 33.0a1 (06/16)
Comment on attachment 8437408 [details] [diff] [review] Capitalize first letter of entries in locale switching UI. v1 Aurora approval granted. >The approach Axel and I decided to take: we'll land this, and then have a >follow-up to add an exceptions mechanism. That gives us a hook for Zulu and >friends, and also provides an extension point for fake developer languages >should we need them. Do you already have a bug on file and is it your intention to include the exceptions mechanism in 32 as well?
Attachment #8437408 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Lawrence Mandel [:lmandel] from comment #18) > Do you already have a bug on file... Yup: Bug 1024095, depends on this one. > and is it your intention to include the > exceptions mechanism in 32 as well? No. AFAIK we don't have any locales in 32 (or 33) that would require it. No point in taking the risk or taking on the work!
Tested with: Device: Samsung Galaxy Nexus OS: Android 4.2.1 The list has entries in upper case, so verified fixed on: Build: Firefox for Android 32 Beta 1
You need to log in before you can comment on or make changes to this bug.