Closed Bug 815852 Opened 12 years ago Closed 12 years ago

On settings app launch, en-US strings appear briefly before correctly localized strings

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Settings
Platform:
ARM
Gonk (Firefox OS)
Type:
defect

Tracking

(blocking-basecamp:-, b2g18+ fixed)

Status:
RESOLVED FIXED
Milestone:
B2G C3 (12dec-1jan)
Project Flags:
blocking-basecamp -
Tracking Flags:
Tracking Status
b2g18 + fixed

People

(Reporter: Margaret, Assigned: kaze)

References

Details

(Whiteboard: [target:12/21])

Attachments

(5 files, 1 obsolete file)

Remove all static text content from the Settings app
(deleted), text/html
Details
POC
(deleted), patch
Details | Diff | Splinter Review
patch proposal
(deleted), text/html
ochameau
: review+
Details
follow-up
(deleted), text/html
vingtetun
: review+
Details
follow-up
(deleted), text/html
vingtetun
: review+
Details 
STR:
-Set your phone's language to something other than English
-Kill the settings app
-Reopen the settings app

We don't wait for the 'localized' event before showing the content of the settings app, and there's a considerable delay before we end up showing the localized text. This isn't noticeable for en-US because we have en-US strings baked into the HTML (see bug 812993), but it looks really bad in other languages.

The improvements I was looking into in bug 809600 make l10n fast enough that this isn't noticeable, but I'm not sure that those will land.

Regardless, I think we mainly should be waiting for the 'localized' event before showing the user any text to make sure it's in the correct language.
blocking-basecamp: ? → +
Priority: -- → P1
Assignee: nobody → bhsubram
I would say the solution here involves replacing the strings at build time and do not use l10n.js unless the language is not the default one.
bhsubram you have been assigned but we didn't discuss the solution. Can we met on IRC to see how this bug can be resolved properly?
Flags: needinfo?(bhsubram)
Vivien: Thank you for your timely offer to discuss. I was planning to start with Margaret and coordinate a discussion this week as this bug and the related bug 816151 need consensus from the group on the approach/solution. I am available 11/3 starting at 8.00 am Pacific. Let me know.
Flags: needinfo?(bhsubram)
Margaret > yes we stopped listening to this event on purpose, as we were trying to load the Settings as fast as possible. When we did this we were expecting to have pre-processed HTML files soon… which isn’t ready yet.

So in the short term we should either empty all text contents from HTML files (as you suggested!) or hide the UI until the 'localized' event is fired (which would be easy).

I think it would be a major mistake to make Gaia any more English-centric by ensuring that the HTML file text contents match the en-US locale resources: it would mask the loading/parsing/reflow time caused by localization to most dogfooders. It’s already very difficult to raise UX attention on issues that are specific to non-English locales, let’s not make it worse please.

So if we can’t pre-translate HTML files for all supported locales in the near term, I’d love to apply your suggestion and remove all text contents from existing HTML files in all Gaia apps.
Mass Modify: All un-milestoned, unresolved blocking-basecamp+ bugs are being moved into the C3 milestone. Note that the target milestone does not mean that these bugs can't be resolved prior to 12/10, rather C2 bugs should be prioritized ahead of C3 bugs.
Target Milestone: --- → B2G C3 (12dec-1jan)
Whats the status here? Fabien?
Andreas, I’m working on the solution Vivien mentions in comment #2.

Pre-building apps for pt-BR will be easy. Applying this to the Settings app will be a little less trivial because of the lazy-loading hack, but easy too.

The key part will be to modify the app:// protocol in order to pre-build *all* supported locales (e.g. en-US + pt-BR + es for v1.0) and load the appropriate HTML file. Taking the Settings app as an example, the idea would be:
 • at build time:
    - remove all localized strings from index.html
    - make index.html.en-US, index.html.es, index.html.pt-BR
    - include all these index.html* files in the application zip archive
 • at run time, if the current locale is pt-BR:
    - app://settings.gaiamobile.org/index.html would point to index.html.pt-BR if found and l10n.js would not have anything to do on the document at startup;
    - if not found (or when switching locale), the index.html file would be used and l10n.js would translate it using the appropriate *.properties files.

I have no idea yet if the app:// trick is going to be easy or not: if anyone wants to work on this part while I’m doing the build-time part, that’d be welcome.
bhsubram: would this approach be OK for you? Do you think we could work together me on this?
(In reply to Fabien Cazenave [:kaze] from comment #7)
> Andreas, I’m working on the solution Vivien mentions in comment #2.

You may want to check with jhford to see if you can reuse any of the stuff he did in bug 810448.

> Pre-building apps for pt-BR will be easy. Applying this to the Settings app
> will be a little less trivial because of the lazy-loading hack, but easy too.
> 
> The key part will be to modify the app:// protocol in order to pre-build
> *all* supported locales (e.g. en-US + pt-BR + es for v1.0) and load the
> appropriate HTML file. Taking the Settings app as an example, the idea would
> be:
>  • at build time:
>     - remove all localized strings from index.html
>     - make index.html.en-US, index.html.es, index.html.pt-BR
>     - include all these index.html* files in the application zip archive
>  • at run time, if the current locale is pt-BR:
>     - app://settings.gaiamobile.org/index.html would point to
> index.html.pt-BR if found and l10n.js would not have anything to do on the
> document at startup;
>     - if not found (or when switching locale), the index.html file would be
> used and l10n.js would translate it using the appropriate *.properties files.
> 
> I have no idea yet if the app:// trick is going to be easy or not: if anyone
> wants to work on this part while I’m doing the build-time part, that’d be
> welcome.

Fabrice has a patch that does this in bug 811540.

Also, you may want to follow up with newsgroup discussion about this:
https://groups.google.com/d/topic/mozilla.dev.gaia/2eEY7wpaidg/discussion
Hi :kaze,

I don't think bhsubram is available to work on this any longer; reassigning to you instead.
Assignee: bhsubram → kaze
(In reply to Fabien Cazenave [:kaze] from comment #7)
> Andreas, I’m working on the solution Vivien mentions in comment #2.
> 
> Pre-building apps for pt-BR will be easy. Applying this to the Settings app
> will be a little less trivial because of the lazy-loading hack, but easy too.

Kaze, just to clarify. There are two proposed solutions here:

1) Build static per-locale HTML files with strings inlined in DOM nodes
2) Build static per-locale HTML files with inline l10n resources in <head/>

I'd suggest we go for approach 2) because it keeps the result HTML code in line with source HTML and limites the hackery that we need here.

Also, what about removing en-US text from nodes?
(In reply to Zbigniew Braniecki [:gandalf] from comment #11)
> Kaze, just to clarify. There are two proposed solutions here:
> 
> 1) Build static per-locale HTML files with strings inlined in DOM nodes
> 2) Build static per-locale HTML files with inline l10n resources in <head/>
> 
> I'd suggest we go for approach 2) because it keeps the result HTML code in
> line with source HTML and limites the hackery that we need here.

I’m not sure to understand your point, but I’m afraid we *really* need the first approach anyway. The real issue with client-side localization is the reflow — not the parsing.

Micro-template engines like mustache.js and handlebar.js use the same trick: they pre-build HTML files whenever possible. Being able to render these templates at run-time is nice and greatly eases the development, but it’s sub-optimal performance-wise. That’s the same for client-side l10n localization, and performances are critical on the targeted devices.

However, we might want to inline the l10n resource in the <head /> of the pre-built file, too: that would avoid some XHR, which is always a good thing — but the benefit is only for dynamically built strings.

> Also, what about removing en-US text from nodes?

It won’t solve any issue (still a lot of reflows — possibly *more* reflows) but it would be easier on the eye. That’s part of the plan I propose in comment #7.
(In reply to Margaret Leibovic [:margaret] from comment #9)
> (In reply to Fabien Cazenave [:kaze] from comment #7)
> You may want to check with jhford to see if you can reuse any of the stuff
> he did in bug 810448. 

Definitely. :-)

> > I have no idea yet if the app:// trick is going to be easy or not […]
> Fabrice has a patch that does this in bug 811540.

Then we’re all set! Thanks for the pointer.
Whiteboard: [target:12/21]
(In reply to Fabien Cazenave [:kaze] from comment #12)
> I’m not sure to understand your point, but I’m afraid we *really* need the
> first approach anyway. The real issue with client-side localization is the
> reflow — not the parsing.

Sure, that's why you want to inject the translation before the first draw.
 
> Micro-template engines like mustache.js and handlebar.js use the same trick:
> they pre-build HTML files whenever possible. Being able to render these
> templates at run-time is nice and greatly eases the development, but it’s
> sub-optimal performance-wise. That’s the same for client-side l10n
> localization, and performances are critical on the targeted devices.

Do you have any numbers for that?
 
> > Also, what about removing en-US text from nodes?
> 
> It won’t solve any issue (still a lot of reflows — possibly *more* reflows)

I believe you can avoid reflows if you translate nodes early. In particular, you can translate nodes on DOMContentLoaded (or even earlier) and this will happen before first paint and in result avoid the reflow.
This is not a patch: this is just a git branch to test what happens if the Settings app HTML document doesn’t have any static text content. As you can see, it doesn’t solve any issue at all — on the contrary.
yeah, that's because you first load resources and then fire localization independently of DOM preparation status. In result you almost always localize nodes after firstPaint.

But since one of the proposals here is to move localization into HTML file, I believe that if you move the resource inline into HTML file and fire localization at DOMContentLoaded or use mutation observers on document.documentElement you will in fact translate the nodes before firstPaint thus avoiding flickering and making firstPaint numbers more realistic (ie. incorporating l10n time)
(In reply to Zbigniew Braniecki [:gandalf] from comment #14).
> > Micro-template engines like mustache.js and handlebar.js use the same trick:
> > they pre-build HTML files whenever possible. Being able to render these
> > templates at run-time is nice and greatly eases the development, but it’s
> > sub-optimal performance-wise. That’s the same for client-side l10n
> > localization, and performances are critical on the targeted devices.
> 
> Do you have any numbers for that?

LTIC, if we wait for the `localized' event to be fired before displaying <body>, it caused a 300 to 600 ms delay. In the current Settings app we don’t wait for this `localized' event, that’s why we see this flickering on the device.

> > > Also, what about removing en-US text from nodes?
> > 
> > It won’t solve any issue (still a lot of reflows — possibly *more* reflows)
> 
> I believe you can avoid reflows if you translate nodes early. In particular,
> you can translate nodes on DOMContentLoaded (or even earlier) and this will
> happen before first paint and in result avoid the reflow.

See branch above: removing en-US text nodes doesn’t solve any issue. I’d even say it makes the experience much worse.

Our l10n library already translate nodes as early as possible (on DOMContentLoaded). We tried other approaches to translate on parsing time long ago, but it didn’t bring any significant improvement. More generally: on faster devices (especially on the desktop) it’s rather easy to translate the whole HTML document before the first paint, but we never achieved this on the Otoro / Unagi devices.

Nothing will be faster/smoother than pre-processing all HTML files anyway. We can still try other approaches for v2 — native DOM-based templating comes to mind, and it would address more than just l10n — but at this stage of the project we really should look for the fastest possible way to translate a page.
(In reply to Zbigniew Braniecki [:gandalf] from comment #16)
> yeah, that's because you first load resources and then fire localization
> independently of DOM preparation status. In result you almost always
> localize nodes after firstPaint.

Gandalf, really: we tried it already. That was how the Settings app worked until ~2 months ago. As I write above, waiting for the `localized' event avoided some reflows but it still had a very significant impact on the loading time.
I’m not gonna make one branch per argument. ;-)

> But since one of the proposals here is to move localization into HTML file,
> I believe that if you move the resource inline into HTML file and fire
> localization at DOMContentLoaded or use mutation observers on
> document.documentElement you will in fact translate the nodes before
> firstPaint thus avoiding flickering and making firstPaint numbers more
> realistic (ie. incorporating l10n time)

We tried that, too. It kinda works on the desktop, but on the device there’s no significant benefit.

Can you at least agree that nothing will be faster/smoother than pre-processing all HTML files?
(In reply to Fabien Cazenave [:kaze] from comment #18)
> LTIC, if we wait for the `localized' event to be fired before displaying <body>,
> it caused a 300 to 600 ms delay. In the current Settings app we don’t wait for
> this `localized' event, that’s why we see this flickering on the device.

That matches my experience. Average time of the whole l10n of Settings app on Unagi is avg 412ms with stdev of 58.
Out of that, major bulk is taken by resource loading. It's parallel, but in total on Settings app asynchronous file loading (ini files) takes 60ms per file and synchronous (.properties files) over 100ms per file (there are 5 of them).

node localization of Settings app, including operations in l10n lib and on DOM nodes takes a total toll of 50ms only. While the time that layout and CSS takes between when DOM is ready and when firstPaint happens is around 700ms.

> Can you at least agree that nothing will be faster/smoother than
> pre-processing all HTML files?

Sure. There's nothing faster than inlining and prebuilding static HTML documents. Maybe a JPEG with a map to click through ;)

I'm sad to hear you didn't get the mutation observers nor early node l10n working, but correct me if I'm wrong - you did try that with remote resources, right?

At the moment loading resources takes a lot of time and my numbers from performance timings suggest that loading of your .properties files which blocks parsing and node translation takes more time than DOM parsing.

What I'm suggesting here is that if we remove the resource loading phase from the picture, you can start localizing earlier and in result avoid the flicker without hacks like that.

Are you saying that you already tried that?
(In reply to Zbigniew Braniecki [:gandalf] from comment #19)
> (In reply to Fabien Cazenave [:kaze] from comment #18)
> > Can you at least agree that nothing will be faster/smoother than
> > pre-processing all HTML files?
> 
> Sure. There's nothing faster than inlining and prebuilding static HTML
> documents. Maybe a JPEG with a map to click through ;)

That reminds me of the good old canvas vs. SVG days…
Since you admit this is the fastest approach, let’s do it.

> What I'm suggesting here is that if we remove the resource loading phase
> from the picture, you can start localizing earlier and in result avoid the
> flicker without hacks like that.
> 
> Are you saying that you already tried that?

Margaret did, during the SF work week. The numbers she got from the firstPaint metrics were impressive, but there was no noticeable benefit on the eye.
BTW, just to be clear: avoiding the flickering effect is trivial, we could just wait for the `localized' event before displaying the <body> content. Again, that’s what we did until recently.

Our problem here is to improve the startup time of all apps, and obviously we could win a *lot* of time by pre-processing HTML files.
(In reply to Fabien Cazenave [:kaze] from comment #20)
> > Sure. There's nothing faster than inlining and prebuilding static HTML
> > documents. Maybe a JPEG with a map to click through ;)
> 
> Since you admit this is the fastest approach, let’s do it.

I believe that what you do here is an informal fallacy.
 
> > Are you saying that you already tried that?
> 
> Margaret did, during the SF work week. The numbers she got from the
> firstPaint metrics were impressive, but there was no noticeable benefit on
> the eye.

Does it mean that what she tried did not remove the flicker? In particular, did her experiment fire node translation on DOMContentLoded, interactive state, did she use mutation observers or some other method?

> BTW, just to be clear: avoiding the flickering effect is trivial, we could just wait for the `localized' event before displaying the <body> content. Again, that’s what we did until recently.

This is the worst way to avoid flicker.

> Our problem here is to improve the startup time of all apps, and obviously we could win a *lot* of time by pre-processing HTML files.

First, I'd appreciate if you talk numbers when you're talking performance. I'm much less sure how much of a win the pre-processing would be once you inline resources. I don't doubt that there will be a win, it's just that it may be very small.

As Vivien pointed out in the bug about CSS/JS minification, there is a cost of manipulating the app codebase at build time and it will increase the complexity of the whole system which in long run may cost us much more than the win we will get out of the pre-processing.

My recommendation is to wait until we get CSP numbers down, startup animation cost down, and the new app:// loading model and review the exact performance numbers once again.
Next, I'd suggest an attempt to inline .properties file in <script/> tag and fire node translation at DOMContentLoaded or use mutation observer on document.documentElement.
(In reply to Fabien Cazenave [:kaze] from comment #20)

> > What I'm suggesting here is that if we remove the resource loading phase
> > from the picture, you can start localizing earlier and in result avoid the
> > flicker without hacks like that.
> > 
> > Are you saying that you already tried that?
> 
> Margaret did, during the SF work week. The numbers she got from the
> firstPaint metrics were impressive, but there was no noticeable benefit on
> the eye.

Actually, it didn't really change the first paint numbers, but it did improve the flickering issue because we were doing the translating before first paint, like Gandalf mentioned. Right now the first paint numbers aren't a good metric, because that first paint is a paint of the en-US strings.

I found that doing pre-processing, but in a way that just inlined the l10ndata, avoided the flicker associated with replacing the strings. Gandalf is right that the bottleneck is loading the resources, so avoiding that might be all we need. I would encourage you to make a build with the patches from bugs 810448, 810450, and 811540, and see if that fixes this issue. The main reason those changes didn't land was concern over whether or not we should be hacking the app:// protocol, and it sounds like we want to go forward with that now.
> My recommendation is to wait until we get CSP numbers down, startup
> animation cost down, and the new app:// loading model and review the exact
> performance numbers once again.

I really don’t think we can afford to wait: there are four weeks left before the release. I’d be much more open to experiments *after* the v1.0 release.

This is a serious performance issue and I’d much prefer coding than discussing on a bugzilla, so unless you can propose a patch very quickly, I’m calling for our tech lead’s decision on this.
Flags: needinfo?(21)
(In reply to Margaret Leibovic [:margaret] from comment #23)
> I found that doing pre-processing, but in a way that just inlined the
> l10ndata, avoided the flicker associated with replacing the strings. Gandalf
> is right that the bottleneck is loading the resources, so avoiding that
> might be all we need. I would encourage you to make a build with the patches
> from bugs 810448, 810450, and 811540, and see if that fixes this issue. The
> main reason those changes didn't land was concern over whether or not we
> should be hacking the app:// protocol, and it sounds like we want to go
> forward with that now.

OK, I’ll do that (after I get some sleep). I might be missing an obvious point here.

Gandalf, I removed all en-US strings as you suggested in comment #13, so feel free to fork my branch to propose a patch.
Attached patch POC (deleted) — — Splinter Review 
Ok, I copied approach l20n takes into the kaze's .properties based lib here's the POC.

What it does is:

 - Replaces <links/> with <script/> nodes and inlines .properties content.
 - Modifies the l10n.js to load content directly from the <script/> tags

With this patch I can no longer reproduce the flicker, since the combined time of parsing + node translation is shorter than DOM layout so it happens before the firstPaint.

I tested it multiple times and with your branch and with this POC and settings.en-US.properties inlined in index.html I'm constantly getting flicker free user experience.

Kaze: let me know what you think about this. I had only a few minutes so I didn't really play nicely to the codebase, but I believe that the real patch should allow for both methods (remote and inline resource loading) and this way we can introduce a least invasive build time method of plain l10n res inlining.
(and then remove the method as fast as we get resource loading time under control of course. Any resource inlining is an unwanted dirty hack, no matter what)
Kaze: based on your actions in https://github.com/mozilla-b2g/gaia/pull/7024 I sense that you decided to move forward with pre-processing whole HTML documents at build time, am I right here?
You’re right.

(In reply to Zbigniew Braniecki [:gandalf] from comment #27)
> (and then remove the method as fast as we get resource loading time under
> control of course. Any resource inlining is an unwanted dirty hack, no
> matter what)

That’s the thing: we both agree that inlining resource is a dirty hack, but pre-localizing HTML files makes sense to me — not only for Gaia, but also for web apps in general (SEO, accessibility…).

The difficult part here is to process HTML files with our build system — pre-translating the HTML content is completely straight-forward. I could inline l10n resouces during this build process if some apps can benefit from this, but I’ll probably use a JSON format rather than the *.properties one — that’s both easier and faster.
Depends on: 821691
(In reply to Fabien Cazenave [:kaze] from comment #29)
> You’re right.

I raised concerns about the solution you were leaning toward, you asked me to prove my approach, I did, and you then ignored it and went forward without responding to either my concerns, my proposal or at all in this thread.

I believe that this behavior is fundamentally against the way Mozilla project culture.

(In reply to Fabien Cazenave [:kaze] from comment #29)
> That’s the thing: we both agree that inlining resource is a dirty hack, but
> pre-localizing HTML files makes sense to me — not only for Gaia, but also
> for web apps in general (SEO, accessibility…).

Are you suggesting that pre-compilation is a dirty-hack or is it a long-term strategy that you'd recommend to gaia-app authors and web authors in general?
 
> The difficult part here is to process HTML files with our build system —
> pre-translating the HTML content is completely straight-forward. I could
> inline l10n resouces during this build process if some apps can benefit from
> this, but I’ll probably use a JSON format rather than the *.properties one —
> that’s both easier and faster.

I completely disagree here.

If we take three proposed approaches here:
1) inject properties into HTML documents and keep the same l10n system
2) convert properties to JSON and inject them into HTML documents
3) pre-compile HTML

Then the first is keeping the whole workflow intact, while workarounding the problem with resource loading time alone. It addresses the exact problem in an easy to reverse way, introducing only one modification.

In the second, you're actually processing properties->json and store this, which makes properties file not a data storage format anymore, but rather a human interface to another format. That introduces a new intermediate layer to the system.

In the third, you actually introduce the whole new layer of document storage, pre-compiled. That modifies the DOM that we're delivering on the product and affects the system (like, document retranslation, reaction to missing entities etc.)

Now, what you're saying here is that approach third is the easier one and safer. I claim the opposite.

Then, you say it's faster. Can you provide numbers for 1 vs. 2 vs. 3?

I believe that you are introducing serious shift in the product localizability that affects many elements, in order to workaround a single issue that can be addressed with much less invasive change.
I also believe that the difference between those three methods in performance impact should be minimal, depending on the exact code between 30-60ms per app.
That's why I advise to minimize the amount of changes taken to address it while keeping app modularity intact.
Driver retriage: If there's a low-risk patch, we'll take it on approval. However, we decided we'll not hold the whole release for this.

I recommend that you guys have a phone conference to come to a conclusion on the right fix approach. Once all the P1s are fixed, that is.
blocking-basecamp: + → -
Attached file patch proposal (obsolete) (deleted) — 
NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): client-side localization
User impact if declined: long startup time and flickering on startup
Testing completed: manual
Risk to taking this patch (and alternatives if risky): rather low, as the l10n.js library hasn’t been modified much.
Attachment #695323 - Flags: review?
Attachment #695323 - Flags: approval-gaia-master?(21)
Flags: needinfo?(21)
This patch (attachment 695323 [details]) relies on the work that has been done for bug 821691: all Gaia applications are pre-translated for the default locale (see the GAIA_DEFAULT_LOCALE build parameter), and all l10n resources are inserted as an inline JSON object.

Expected benefits:
 • no more XHR to load l10n resources, which should allow to display localized apps without any flickering with any locale;
 • the *.ini and *.properties resources aren’t zipped any more so that webapps don’t become heavier on the device storage;
 • the line numbers in the HTML file should not be changed — external l10n resource links are commented out, and the JSON dictionary is added at the end of the document;
 • very slight modification of l10n.js, and no modification at all on Gaia apps — low risk!

Feedback very welcome.
Attachment #695323 - Flags: review? → review?(poirot.alex)
Comment on attachment 695323 [details]
patch proposal

Can you please at least fix the profile cloberring issue ?
Otherwise I tested it and it works fine. We no longer see any translation blink effect on app startup, even when switching to another locale other than the default one. Note that the blink effect already disappear when landing bug .

Otherwise here is what I had in mind in order to simplify the comprehension and the implementation of dictionary generation:
Do something like this in webapp-l10n:
var mozL10n = l10nTarget.mozL10n;
for (let lang in locales) {
  var dictionary = l10nDictionary.locales[lang] = {};
  var resources = docElt.querySelectorAll('link[type="application/l10n"]');
  var content = 
  for (let i = 0; i < resources.length; i++) {
    var res = resources[i];
    var content = somehowGetL10nResourceFileContent(res.href));  
    mozL10n.parseResourceContent(dictionary, lang, content);
  }
}

But it requires way more modification in l10n lib in order to split `parseResource` in two. Something similar to this:
function parseResource(href, lang, successCallback, failureCallback) {
  loadResource(href, function(response) {
    parseResourceContent(gL10nData, lang, response);
  }, successCallback, failureCallback, gAsyncResource...);
}

I let you free to land as-is, as it works fine and is still readable.
I let you decide if it worth it given the more complex l10n lib modification.
That's the kind of thing we can also do in followup.
Attachment #695323 - Flags: review?(poirot.alex) → review+
(In reply to Alexandre Poirot (:ochameau) from comment #34)
> Can you please at least fix the profile cloberring issue ?

Thanks, I’ve just updated the pull request to use the `?reload=' trick.

> I let you free to land as-is, as it works fine and is still readable.
> I let you decide if it worth it given the more complex l10n lib modification.
> That's the kind of thing we can also do in followup.

I agree I’d prefer to have a synchronous way to process all l10n resources, though I’m a bit reluctant to modify l10n.js at this stage of the project.

If I can come up with a clean / simple patch for that I’ll update the pull request tomorrow, otherwise a follow-up sounds wise.

Thanks again for your review, merry X-mas! :-)
Comment on attachment 695323 [details]
patch proposal

https://github.com/mozilla-b2g/gaia/commit/7378e497d00a035ff337c4018f3fa1753bf684bf

Sounds good to me.
Attachment #695323 - Flags: approval-gaia-master?(21) → approval-gaia-master+
It looks like this may have caused red builds on inbound:
https://tbpl.mozilla.org/php/getParsedLog.php?id=18277399&tree=Mozilla-Inbound
I backed this out because of the test failures on TBPL:
https://github.com/mozilla-b2g/gaia/commit/3ecc1f828561b1be1588b0e185f9a0538814c8a6
Matt, could you tell me if that works with a fresh profile by any chance?
It looks like TBPL doesn't build with a fresh profile:

   14:00:50     INFO -  test -d profile || mkdir -p profile

I’m not sure it’d be an acceptable solution to require a clobber build, though.
Attached file patch proposal (deleted) — 
Same patch, with more debugging info. This time we should have more support from #ateam if tinderbox gets red.
Attachment #695323 - Attachment is obsolete: true
Attachment #699399 - Flags: review?(poirot.alex)
Comment on attachment 699399 [details]
patch proposal

Still works fine for me, let see how this works on tinderbox.
Attachment #699399 - Flags: review?(poirot.alex) → review+
https://github.com/mozilla-b2g/gaia/commit/e342e766c1289023d93c53714cc48f303debe41c

at least we should know which file is causing problems (hopefully)
OK, the error is caused by the multilocale build:
https://tbpl.mozilla.org/php/getParsedLog.php?id=18606735&tree=Mozilla-Inbound&full=1#error0

The `shared/resources/languages-dev.json' file is missing, according to the error report. Investigating.
Attached file follow-up (deleted) — 
The previous patch created an error with multilocale builds because of LOCALES_DIR — which has a relative path by default, and an absolute one when built with multilocale. Here’s a fix.
Attachment #699479 - Flags: review?(21)
Comment on attachment 699479 [details]
follow-up

Post mortem r+ :)
Attachment #699479 - Flags: review?(21) → review+
Attached file follow-up (deleted) — 
tested on the device, with and without multilocale.
Attachment #699496 - Flags: review?(21)
Comment on attachment 699496 [details]
follow-up

Zombie review+.
Attachment #699496 - Flags: review?(21) → review+
https://github.com/mozilla-b2g/gaia/commit/02a8f5865f5f4f3de3fa7caffb55d20ff530fe09
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: