817286 - [l10n] l10n.js failed if locale ini imports non-exist propertie files

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia, defect, P1)

Description

[John Shih]

## Environment :
Unagi phone, build 2012-11-30
Build info: 
gaia master : ccf5f69acf7cb9f1831ecc69d7c518c243c74c3a
mozilla-beta : 6c14548dbd4e

## Repro :
1. Enter settings app
2. Enable the Language page
3. Launch the language selection page
4. Press Hebrew first, verify the layout will left-right reverse
5. Press Francis or Mandarin 

## Expected:
* The layout will reverse back

## Actual:
* Didn't reverse back

## Note:
* Only English works normally

Comment 1

[John Shih]

It also happened in FTU

Comment 2

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

Hebrew should not be in scope for v1.

Comment 3

[Steve Chung [:steveck]]

I discovered the root cause about this issue. I can take this one if it becomes BB+ issue. Thanks

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Steve Chung from comment #3)
> I discovered the root cause about this issue. I can take this one if it
> becomes BB+ issue. Thanks

Please document your progress by writing it down here, or at least upload an WIP patch.

Comment 5

[David Scravaglieri [:scravag]]

Not on V1

Comment 7

[Steve Chung [:steveck]]

The root cause is because the localize event does not dispatch when locale file path set in ini but no actual locale file matched. It will make localization failed if we forget to update the responding locale file that set in ini. Although Fr and zh-TW is not the target language for V1, we should have better error handling for l10n parsing because such scenario could be reproduced easily in the future.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Update title, see comment 7.

Pike, do you think fragile l10n bug like this should block?

Comment 9

[Axel Hecht [:Pike]]

Yes, we need this. We need confidence that the builds we do are stable against broken localizations. Right now, on any issue with a localized builds, the emails end with a pointer "and then there's this bug, who knows if it's affecting it".

Comment 10

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

BB-. Do we have a concrete example of where this is a problem for the langauges that we are shipping with?

Comment 11

[Axel Hecht [:Pike]]

I can't commit to provide working localizations for either 1.0 nor 1.1 without this bug being fixed. Given that we're taking string changes on a daily basis still, there's no point to restrict the impact of this bug on current problems, but any of the pending changes can break the build to be untestable.

Comment 12

[Ben Francis [:benfrancis]]

Would removing the non-working locales from the language list in Settings fix this problem?

Comment 13

[Fabien Cazenave [:kaze]]

Attachment: quick and dirty patch

Steve, please let me know if you have a better patch in mind.

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): ?
User impact if declined: some apps (e.g. FTU) are broken if any l10n resource is missing 
Testing completed: manual
Risk to taking this patch (and alternatives if risky): that’s just a try…catch

Comment 14

[Axel Hecht [:Pike]]

(In reply to Ben Francis [:benfrancis] from comment #12)
> Would removing the non-working locales from the language list in Settings
> fix this problem?

With the constraint that we won't be providing localizations, like I alerted at in my previous comment.

Glancing at the pull request, that should also address the issue.

Comment 15

[Steve Chung [:steveck]]

Comment on attachment 690892 [details]
quick and dirty patch

I think using try/catch is also the method I will apply in this fixing(for non-existing URL), thanks!

Comment 16

[Ben Francis [:benfrancis]]

Trage: Steve can you explain the problem more fully and why it prevents localisation from working. From the description it sounds like there's only an issue if the .properties file is missing.

Comment 17

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Is this a dup of bug 816978? We could just land the bug there since it's already blocking+'d

Comment 18

[Fabien Cazenave [:kaze]]

Right, Daniel just pinged me about it. Can someone put a bb+ label on this one please?

Comment 20

[Ben Francis [:benfrancis]]

Thanks for the info Tim

Comment 21

[Guillermo López :willyaranda (probably SLOW response)]

I just installed master gaia on unagi and Spanish is working perfectly, thanks!

Comment 22

[Fabien Cazenave [:kaze]]

Glad to read this, thanks for your early feedback!

Comment 23

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Ben Francis [:benfrancis] from comment #20)
> Thanks for the info Tim

Cool, I just realized I needinfo?'d nobody!

Comment 24

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Steve Chung from comment #15)
> Comment on attachment 690892 [details]
> quick and dirty patch
> 
> I think using try/catch is also the method I will apply in this fixing(for
> non-existing URL), thanks!

Kaze, I would say this deserves filing a platform bug and a mention of that bug number in the code. Would you file that bug please?

Comment 25

[Deepa Subramanian]

Verified on Unagi device Build ID: 20130103070201