Closed Bug 1013226 Opened 5 years ago Closed 5 years ago

Add Indic locales to maemo-locales

Categories

(Firefox for Android :: General, defect)

31 Branch
All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 31
Tracking Status
firefox30 --- unaffected
firefox31 --- fixed
relnote-firefox --- 31+

People

(Reporter: gueroJeff, Assigned: gueroJeff)

References

Details

Attachments

(1 file, 1 obsolete file)

Please add as, bn-IN, gu-IN, hi-IN, kn, mai, ml, mr, or, pa-IN, ta, te to mobile/android/locales/maemo-locales to be included in the Fennec 31. Please add to Aurora and port to Beta at merge.
Attached patch add_indic.patch (obsolete) — Splinter Review
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or IDL/UUID changes made by this patch:
Attachment #8425410 - Flags: review?(mark.finkle)
Attachment #8425410 - Flags: approval-mozilla-aurora?
Comment on attachment 8425410 [details] [diff] [review]
add_indic.patch

I assume this bumps up the APK size noticeably.
Attachment #8425410 - Flags: review?(mark.finkle) → review+
(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 8425410 [details] [diff] [review]
> add_indic.patch
> 
> I assume this bumps up the APK size noticeably.

We'll have to watch it, but if the estimate of 3MB per every 50 locales is still true, this shouldn't even hit a 1MB increase.
Flags: in-moztrap?(fennec)
(In reply to Jeff Beatty [:gueroJeff] from comment #1)
> Created attachment 8425410 [details] [diff] [review]
> add_indic.patch
> 
> [Approval Request Comment]
Jeff, could you fill the uplift request template? Thanks
Flags: needinfo?(jbeatty)
(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> (In reply to Jeff Beatty [:gueroJeff] from comment #1)
> > Created attachment 8425410 [details] [diff] [review]
> > add_indic.patch
> > 
> > [Approval Request Comment]
> Jeff, could you fill the uplift request template? Thanks

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 987653, bug 987633, bug 987655, bug 987657, bug 987659, bug 987662, bug 987647, bug 987638, bug 987628, bug 960058, bug 987642, bug 987645
User impact if declined: Apprx. 135K users in India will be unable to use Fennec in their language.
Testing completed (on m-c, etc.): m-a
Risk to taking this patch (and alternatives if risky): Will likely increase apk size, but by less than 1MB
String or IDL/UUID changes made by this patch: N/A
Flags: needinfo?(jbeatty)
Thanks. Does this need to land in m-c first?
(In reply to Sylvestre Ledru [:sylvestre] from comment #6)
> Thanks. Does this need to land in m-c first?

No, only a handful of localizers perform their work on m-c. This only needs to be on m-a and then ported to m-b on merge day. Thank you.
Attachment #8425410 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/da83ebcf2099
Assignee: nobody → jbeatty
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed-aurora]
Target Milestone: --- → Firefox 31
This broke Android nightly packaging. Backed out.
https://hg.mozilla.org/releases/mozilla-aurora/rev/0cfdaf62328a

https://tbpl.mozilla.org/php/getParsedLog.php?id=40184234&tree=Mozilla-Aurora

09:30:59     INFO -  invalid resource directory name: /builds/slave/m-aurora-and-a6-ntly-000000000/build/obj-firefox/mobile/android/base/res/values-mai
Status: RESOLVED → REOPENED
Flags: needinfo?(jbeatty)
Resolution: FIXED → ---
Not sure why this broke, the directory exists http://hg.mozilla.org/releases/l10n/mozilla-aurora/mai/file/b2abd3161b6e/mobile and according to the Android dev doc, devs can create locale codes for languages and countries that don't exist: http://developer.android.com/reference/java/util/Locale.html
Flags: needinfo?(jbeatty)
FYI, Android nightly retriggers succeeded post-backout.
Can an engineer attempt a build on their own machine to get a more detailed idea of what's breaking here? I'm wondering if this is an issue with the ISO 639-2 code used mai, but Android doc claims to support it, so having an engineer build multilocale with mai would help narrow down the issue.
Flags: needinfo?(mark.finkle)
I am not setup to build multi-locales, but Richard or Nick might. I think we need to test mozilla-aurora here, not mozilla-central.
Flags: needinfo?(rnewman)
Flags: needinfo?(nalexander)
Flags: needinfo?(mark.finkle)
Testing now.
Flags: needinfo?(rnewman)
Flags: needinfo?(nalexander)
I can reproduce this locally.

 0:24.66 invalid resource directory name: /Users/rnewman/moz/hg/mozilla-aurora/objdir-droid/mobile/android/base/res/values-mai
(In reply to Richard Newman [:rnewman] from comment #15)
> I can reproduce this locally.
> 
>  0:24.66 invalid resource directory name:
> /Users/rnewman/moz/hg/mozilla-aurora/objdir-droid/mobile/android/base/res/
> values-mai

Moar logz, plz.  Is this coming from aapt?  That would be unfortunate.
Android only supports two-letter codes in resource directories.

"The language is defined by a two-letter ISO 639-1 language code, optionally followed by a two letter ISO 3166-1-alpha-2 region code (preceded by lowercase "r"). "

http://developer.android.com/guide/topics/resources/providing-resources.html#table2

It's simply not possible to ship `mai` using the Android resource structure, at least using the standard Android packaging tools.
(In reply to Richard Newman [:rnewman] from comment #17)
> Android only supports two-letter codes in resource directories.
> 
> "The language is defined by a two-letter ISO 639-1 language code, optionally
> followed by a two letter ISO 3166-1-alpha-2 region code (preceded by
> lowercase "r"). "
> 
> http://developer.android.com/guide/topics/resources/providing-resources.
> html#table2
> 
> It's simply not possible to ship `mai` using the Android resource structure,
> at least using the standard Android packaging tools.

Parachuting into a mine field here, but my experiments showed that you can define any locale in the APK that you care to.  values-xx_XX is fine, for example.  So can we introduce an indirection table that says 'mai' maps to 'fa_KE'?
(In reply to Nick Alexander :nalexander from comment #16)

> Moar logz, plz.  Is this coming from aapt?  That would be unfortunate.

Yes, this is coming from aapt.

0:24.65 make[4]: Entering directory `/Users/rnewman/moz/hg/mozilla-aurora/objdir-droid/mobile/android/base'
 0:24.66 (skipping file '.mkdir.done' due to ANDROID_AAPT_IGNORE pattern '.*')
 0:24.66 invalid resource directory name: /Users/rnewman/moz/hg/mozilla-aurora/objdir-droid/mobile/android/base/res/values-mai
 0:24.66 make[4]: *** [.aapt.nodeps] Error 1
 0:24.67 make[4]: *** Deleting file `.aapt.nodeps'
 0:24.67 make[4]: Leaving directory `/Users/rnewman/moz/hg/mozilla-aurora/objdir-droid/mobile/android/base'
 0:24.67 make[3]: *** [make-package-internal] Error 2
 0:24.67 make[3]: Leaving directory `/Users/rnewman/moz/hg/mozilla-aurora/objdir-droid/mobile/android/installer'
 0:24.67 make[2]: *** [make-package] Error 2



Here's the failing line:

        AaptGroupEntry group;
        String8 resType;
        bool b = group.initFromDirName(entry->d_name, &resType);
        if (!b) {
            fprintf(stderr, "invalid resource directory name: %s/%s\n", srcDir.string(),
                    entry->d_name);
Oh, Android.

    // locale - language
    if (part.length() == 2 && isalpha(part[0]) && isalpha(part[1])) {
        loc = part;

        index++;
        if (index == N) {
            goto success;
        }
        part = parts[index];
    } else {
        //printf("not language: %s\n", part.string());
    }
Filed <https://code.google.com/p/android/issues/detail?id=69950>.

Next up: build a custom version of `aapt`, see if it produces a working APK.
In the mean time: Jeff, I suggest dropping Maithili from this batch.

We have ways forward -- repackage the APK to rename a fake locale code, use a fake locale code and just make sure it works in the locale picker, fix aapt to allow for three-letter locale codes -- but there's no sense blocking the other new locales on that.
(In reply to Nick Alexander :nalexander from comment #18)

> Parachuting into a mine field here, but my experiments showed that you can
> define any locale in the APK that you care to.  values-xx_XX is fine, for
> example.

There's anecdotal evidence that this breaks at some point along the chain, btw:

http://stackoverflow.com/questions/3950224/custom-locale-in-android

I don't know if we simply follow a different chain and avoid the pain!
I can still repro with the current tools, though. But should be fixed in release... sometime.
Leaving a note for the future: this still fails with the current (19.1.0) build tools.

objdir-droid/config/autoconf.mk
5:AAPT = /Users/rnewman/moz/android/android-sdk-macosx/build-tools/19.1.0/aapt
Comment on attachment 8427336 [details] [diff] [review]
Temporarily exclude Maithili from maemo-locales.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 987653, bug 987633, bug 987655, bug 987657, bug 987659, bug 987662, bug 987647, bug 987638, bug 987628, bug 960058, bug 987645
User impact if declined: Apprx. 135K users in India will be unable to use Fennec in their language.
Testing completed (on m-c, etc.): m-a
Risk to taking this patch (and alternatives if risky): Will likely increase apk size, but by less than 1MB. With the removal of mai, bustages should no longer be an issue.
String or IDL/UUID changes made by this patch: N/A
Attachment #8427336 - Flags: review?(mark.finkle)
Attachment #8427336 - Flags: approval-mozilla-aurora?
Attachment #8427336 - Flags: review?(mark.finkle) → review+
Attachment #8427336 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8425410 - Attachment is obsolete: true
https://hg.mozilla.org/releases/mozilla-aurora/rev/b1258410c56d
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.