1344625 - Turn on ENABLE_INTL_API=yes on Android's release build

Status: RESOLVED FIXED

(Core :: JavaScript: Internationalization API, enhancement)

Description

[Makoto Kato [:m_kato]]

I turn off ENABLE_INTL_API=yes on release build of Fennec. This is tracking bug that ENABLE_INTL_API=yes is turned on release build.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Joe, Jet, we've disabled ICU 4 months ago on release of Fennec with an intention to push for moving localization resources out of the build and then enable ICU.
With the recent decision to move the localization refactor to Q3, can we now unblock enabling ICU on release and cleaning up Gecko of off the non-ICU codepaths?
The engineering overhead, maintenance burden and accruing number of `gotchas` due to new APIs relying on ICU and not being present in Fennec is increasing with time.

In bug 1369544 comment 4 :rnewman stated that he's supporting enabling ICU unconditionally.

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

James offered to check for any blockers and see if we can get a green light now

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Yeah, I think we should revisit this. Andreas, Joe, Connor: this will add APK size, but will reduce engineering friction which I think is desperately needed considering current resources dedicated to Fennec. It will also allow downloadable locales, which could reduce some of the APK size increase.

Comment 5

[Andreas Bovens [:abovens]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #4)
> Yeah, I think we should revisit this. Andreas, Joe, Connor: this will add
> APK size, but will reduce engineering friction which I think is desperately
> needed considering current resources dedicated to Fennec. It will also allow
> downloadable locales, which could reduce some of the APK size increase.

How much additional APK size are we talking about?

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

According to bug 1215247 comment 105 the change is 2.7mb. We should win those bytes back by the end of the year with the transition to the new localization API which will allow us to remove l10n data from the apk size (~2-3mb back).

re-requesting approval.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

Andres over email: "Sure, please go ahead."

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

:gandalf, when do you expect to turn this on?

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

As soon as I get go-ahead from :jcheng. I'd like to try to land this in 56 to give us some time to confirm stability before we start removing old-APIs and with/after 57 there's going to be a lot of that (bug 1347507).

Comment 10

[Joe Cheng [:jcheng] (please needinfo)]

hi Zibi, please go ahead and land this feature but let's make sure we continue to work towards downloadable locales so we can reverse the apk size hit. Thanks

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1344625 - Turn on ENABLE_INTL_API=yes on Android's release build.

Review commit: https://reviewboard.mozilla.org/r/158144/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158144/

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Joe Cheng [:jcheng] (please needinfo) from comment #10)
> hi Zibi, please go ahead and land this feature but let's make sure we
> continue to work towards downloadable locales so we can reverse the apk size
> hit. Thanks

We're making good progress on that actually. It's a low priority for now, so I'm making sure not to take time from people working on 57, but here's the progress:

 - MessageContext is in review in bug 1347801
 - L10nRegistry is in review in bug 1333980
 - New langpack build system is in review in bug 1365440
 - New langpack runtime consumption is in development in bug 1365709

I expect them to land before 58, which means that by the time we're done with 57 we can start talking about transitioning toolkit strings to the new API which, in turn, will allow us to use langpacks for those strings in Android and reduce the apk size.

Comment 13

[Makoto Kato [:m_kato]]

Comment on attachment 8887321 [details]
Bug 1344625 - Turn on ENABLE_INTL_API=yes on Android's release build.

https://reviewboard.mozilla.org/r/158144/#review163328

test_interfaces.html etc will be failure on beta and release channel.  So could you add backout patch of bug 1349879?  If adding it, it is OK.

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: Bug 1344625: Revert "Bug 1349879 - Intl object is hidden on Android beta and release channel".

Review commit: https://reviewboard.mozilla.org/r/158164/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/158164/

Comment 15

[Makoto Kato [:m_kato]]

Comment on attachment 8887321 [details]
Bug 1344625 - Turn on ENABLE_INTL_API=yes on Android's release build.

https://reviewboard.mozilla.org/r/158144/#review163338

Comment 16

[Makoto Kato [:m_kato]]

Comment on attachment 8887334 [details]
Bug 1344625: Revert "Bug 1349879 - Intl object is hidden on Android beta and release channel".

https://reviewboard.mozilla.org/r/158164/#review163340

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c9951ce87dc
Turn on ENABLE_INTL_API=yes on Android's release build. r=m_kato
https://hg.mozilla.org/integration/autoland/rev/8bb928212540
Revert "Bug 1349879 - Intl object is hidden on Android beta and release channel". r=m_kato

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5c9951ce87dc
https://hg.mozilla.org/mozilla-central/rev/8bb928212540

Comment 19

[Gerry Chang [:gchang]]

We are about to go to 55 RC. Mark 55 won't fix.

Comment 20

[Florian Scholz (Open Web Docs)]

Developer release notes:
https://developer.mozilla.org/en-US/Firefox/Releases/56#JavaScript

Updated compat tables here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/getCanonicalLocales
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Collator
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Collator/prototype
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Collator/supportedLocalesOf
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Collator/compare
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Collator/resolvedOptions
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat/prototype
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat/supportedLocalesOf
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat/formatToParts
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat/format
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat/resolvedOptions
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat/prototype
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat/supportedLocalesOf
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat/format
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat/resolvedOptions