Steps to reproduce -------------------------------------------------------------------- 0. Make sure Email account was not configured in your device 1. Set your device language to English 2. Launch Email # You will see the New Account page is shown in English. 3. Go to Settings -> Launguage, change language to bn-BD （Bengali）and save 4. Launch Email # New Account page is shown in Bengali. 5. Go to Settings -> Launguage, change language back to English 6. Launch Email --> New Account page is shown in Bengali. Sometimes it shows English, but after kill and relaunch Email, this page is in Bengali. Restarting device will not help. Please see attached video for details.

This is the cookie cache, looks like on restore, we need to be sure the l10n stuff is run on the existing dom? Needs a bit more investigation, which I will likely do as part of bug 1005760, so will mark a dependency on that bug. Would be good to confirm that this happens on master, and not sure what the 1.3t uplift story would be in general for this bug or bug 1005760.

Partner nominate this bug as a mas production blocker now. Please help~

This needs a combination of the fix from bug 1005760 and bug 1034227. Specifically we want both fixes, but not the part of the fix for bug 1034227 that removes the fix from bug 1005760. I'll prepare and test a v1.3t-specific pull request for this. There is no longer a blocking 1.3T? flag that I am able to request, so I'm not sure what to do about getting approval to land such a fix on our v1.3t branch. pcheng, do you know? Does partner-blocker authorize me?

Rachelle/Wayne, could you please help?

Hi James, Could you please help to set the 'blocking 1.3T? flag' for this block issue? Hi Andrew Sutherland, Could you provide the patch so that I can verify it asap and then prepare the pull request for v1.3t after the flag ready? Thanks a lot.

There is no 1.3t flag now.

Keven/Thomas, Could you please help? We need land this PR into v1.3t asap.

Here's the prepared fix. It: - Re-localizes the cookie cache at startup. There will be a slight delay, but we should end up at the correct language after a second or two regardless of any startup races. - Re-localizes the template nodes on language change. Since the email app seems likely to be killed by the user switching to the settings app this fix probably doesn't actually matter, but there's no harm in including it. The specific commit is https://github.com/asutherland/gaia/commit/9bdcd3d93509c319fb0152e9259b5a0b84d08035 Bhavana, can you help clarify how I get approval to land this on v1.3t? Approval request-wise this: - Addresses a start-up time localization problem. - Is assembled from fixes that had landed on the v2.0/v2.1 trunks and had no obvious breakage, although - Has been tested on a Tarako device by me just now. Other people testing can't hurt. - This is not a regression, we've had dynamic language change problems for quite a while. Note that this does not fix all of them on v1.3t! There are some things that do not dynamically localize on trunk still! However, all of the rest of the problems are reliably addressed by restarting and since, as noted, the Tarako device almost certainly can't keep the email app alive while the user is changing the language, these are not likely to be a problem the user is able to reproduce.

(In reply to Andrew Sutherland [:asuth] from comment #9) > Created attachment 8459262 [details] > localize cookie cache on startup and localize template nodes > > Here's the prepared fix. It: > - Re-localizes the cookie cache at startup. There will be a slight delay, > but we should end up at the correct language after a second or two > regardless of any startup races. > - Re-localizes the template nodes on language change. Since the email app > seems likely to be killed by the user switching to the settings app this fix > probably doesn't actually matter, but there's no harm in including it. > > The specific commit is > https://github.com/asutherland/gaia/commit/ > 9bdcd3d93509c319fb0152e9259b5a0b84d08035 > > > Bhavana, can you help clarify how I get approval to land this on v1.3t? > > > Approval request-wise this: > - Addresses a start-up time localization problem. > - Is assembled from fixes that had landed on the v2.0/v2.1 trunks and had no > obvious breakage, although > - Has been tested on a Tarako device by me just now. Other people testing > can't hurt. > - This is not a regression, we've had dynamic language change problems for > quite a while. Note that this does not fix all of them on v1.3t! There are > some things that do not dynamically localize on trunk still! However, all > of the rest of the problems are reliably addressed by restarting and since, > as noted, the Tarako device almost certainly can't keep the email app alive > while the user is changing the language, these are not likely to be a > problem the user is able to reproduce. :asuth, thanks for the detailed analysis, could you also comment on the risk profile of these changes(risk of fallouts,any other know regressions etc) ? I've blocked on it given comment #3, but I'd still like to get a confirmation by wayne that this is a SHOW STOPPER for 1.3T for the partner, as we are only taking those sorts of issues on the 1.3T branch at this point before I approve this.

(In reply to bhavana bajaj [:bajaj] [On PTO until July 27 ] from comment #10) > :asuth, thanks for the detailed analysis, could you also comment on the risk > profile of these changes(risk of fallouts,any other know regressions etc) ? > I've blocked on it given comment #3, but I'd still like to get a > confirmation by wayne that this is a SHOW STOPPER for 1.3T for the partner, > as we are only taking those sorts of issues on the 1.3T branch at this point > before I approve this. I would characterize the risk as non-existent. The patch adds two calls to our mozL10n library's mozL10n.translate method and some very minor book-keeping to make sure one of the set of calls happens on language change. The only code we actually expect to trigger (because Tarako devices don't have a lot of memory) are the cached node localization and the book-keeping. These are both exercised during normal app startup by clicking the email app icon from the home-screen so any problems would be immediately obvious. The only way something bad happens is if the l10n library fails and starts throwing exceptions. We could wrap our new call to mozL10n.translate in such an exception handler, but arguably the l10n library failing isn't something that should happen or something that we want hidden. Specifically, if we add the 'try' and then someone later breaks the l10n library that badly, we probably want it to be obvious that we broke it. Also as noted this patch existed on trunk and was fine and was developed and reviewed by two other developers, so now we've had all three primary email developers' eyes on this.

Hi! Arvin, Please verify patch https://github.com/asutherland/gaia/commit/9bdcd3d93509c319fb0152e9259b5a0b84d08035 from comment #9. Thanks -- Keven

(In reply to Keven Kuo [:kkuo] from comment #12) > Hi! Arvin, > > Please verify patch > https://github.com/asutherland/gaia/commit/ > 9bdcd3d93509c319fb0152e9259b5a0b84d08035 from comment #9. Thanks > > -- > Keven Verify OK. I've checked the patch on the branch v1.3t locally and email app is always shown in the right language no matter what. It indeed exists a little delay under the special case-'set current language as English -- enter email -- home -- change language to France -- hold home to kill email(maybe already killed by LMK) -- enter email', but I think it is perfectly acceptable for language SW is triggered by user themselves. Thanks.

patch provided and verified.

Hi Andrew Sutherland, The Travis CI build failed, could you please help take a look at it? Thanks a lot.

(In reply to helloarvin from comment #15) > The Travis CI build failed, could you please help take a look at it? Sorry for not noting it before; these were contacts app failures and unrelated to the email app changes and are likely intermittent failures. Because our Travis builders have been overwhelmed with jobs I didn't reschedule the jobs since from my memory there are a lot of intermittent integration test failures on v1.3t. And jshint runs are not relevant on v1.3t. I am waiting for confirmation from Wayne as requested by :bajaj in comment 10 before merging the patch.

(In reply to Andrew Sutherland [:asuth] from comment #16) > (In reply to helloarvin from comment #15) > > The Travis CI build failed, could you please help take a look at it? > > Sorry for not noting it before; these were contacts app failures and > unrelated to the email app changes and are likely intermittent failures. > Because our Travis builders have been overwhelmed with jobs I didn't > reschedule the jobs since from my memory there are a lot of intermittent > integration test failures on v1.3t. And jshint runs are not relevant on > v1.3t. > > > I am waiting for confirmation from Wayne as requested by :bajaj in comment > 10 before merging the patch. Get it. Thank you very much.

Sorry for a late response Andrew, been under poor network availability in the last week. Since we have a very low risk patch available let's get it uplifted (and the flags already there, thanks :bajaj!). Thank you Andrew.

landed on gaia/v1.3t: https://github.com/mozilla-b2g/gaia/pull/21960 https://github.com/mozilla-b2g/gaia/commit/62b7b359e59a5edab697b1b3fd123c0a8568444b

(In reply to Andrew Sutherland [:asuth] from comment #19) > landed on gaia/v1.3t: > https://github.com/mozilla-b2g/gaia/pull/21960 > https://github.com/mozilla-b2g/gaia/commit/ > 62b7b359e59a5edab697b1b3fd123c0a8568444b Thanks a lot.

Dear Andrew, The issue also exists in the branch v1.4 after I checked the issue on flame 1.4 and dolphin. Could you please help merge the PR into v1.4?

Comment on attachment 8459262 [details] localize cookie cache on startup and localize template nodes [Approval Request Comment] This has already landed on v1.3t and (effectively) on v2.1 trunk. I am requesting 1.4 approval because of the partner request in comment 21 and this was already a "mass production blocker" nomination per comment 3. I have confirmed an automated cherry-pick of the v1.3t patch works and that the mozL10n.translate API is the correct API call in that version. I am requesting 2.0 approval because it will be very confusing for this problem to reproduce on 2.0 but not 1.3t/1.4/2.1 and it's better to do now than in a few months. I have confirmed an automated cherry-pick of the v1.3t patch and that the mozL10n.translate API continues to be the correct call on v2.0. (That is not the case on v2.1 but we're using the right API call there.) == Risk analysis from comment 9 == - Addresses a start-up time localization problem. - Is assembled from fixes that had landed on the v2.0/v2.1 trunks and had no obvious breakage. - Has been tested on a Tarako device by me just now. Other people testing can't hurt. - This is not a regression, we've had dynamic language change problems for quite a while. Note that this does not fix all of them on v1.3t! There are some things that do not dynamically localize on trunk still! However, all of the rest of the problems are reliably addressed by restarting and since, as noted, the Tarako device almost certainly can't keep the email app alive while the user is changing the language, these are not likely to be a problem the user is able to reproduce. == Risk analysis from comment 11 == I would characterize the risk as non-existent. The patch adds two calls to our mozL10n library's mozL10n.translate method and some very minor book-keeping to make sure one of the set of calls happens on language change. The only code we actually expect to trigger (because Tarako devices don't have a lot of memory) are the cached node localization and the book-keeping. These are both exercised during normal app startup by clicking the email app icon from the home-screen so any problems would be immediately obvious. The only way something bad happens is if the l10n library fails and starts throwing exceptions. We could wrap our new call to mozL10n.translate in such an exception handler, but arguably the l10n library failing isn't something that should happen or something that we want hidden. Specifically, if we add the 'try' and then someone later breaks the l10n library that badly, we probably want it to be obvious that we broke it. Also as noted this patch existed on trunk and was fine and was developed and reviewed by two other developers, so now we've had all three primary email developers' eyes on this.

Hi Andrew, Really appreciate your quick reply and kindly help!

Comment on attachment 8459262 [details] localize cookie cache on startup and localize template nodes 1.4+ as per partner request. 2.0+ as this is a relatively safe functional correctness patch and will give consistency across all branches.

v2.0: https://github.com/mozilla-b2g/gaia/commit/8cf54dbf49d5f8e249f9ac0759f8502bfecda6a4 v1.4: https://github.com/mozilla-b2g/gaia/commit/7d17aa604499a73c97e3314b9ed73bc249b57f88

Dear Andrew, There are other issues of language localization. STR: 1. Open APP Settings, and set language English. 2. Open APP Email, and set up an account. 3. Then you can see the inbox in English. 4. Open APP Settings again, and change language to another language, such as French。 5. Back to Email, you will find inbox still in English. 6. Open setting of email, add another account. 7. You will find login interface still in English. OBSERVED： 8. inbox and login interface still in English. EXPECTED: 9. inbox and login interface should in French.

Andrew, Open the edit mode UI of email. The msg-listedit-header-label still is 'edit' in English after language changing to French. It should be in French. Close the edit mode UI of email, then back to the edit mode UI of email. The msg-listedit-header-label displays normally. We can solve the issue. --- a/apps/email/js/cards/message_list.html +++ b/apps/email/js/cards/message_list.html @@ -20,7 +20,7 @@ <a href="#" class="msg-listedit-cancel-btn"> <span class="icon icon-close"></span> </a> - <h1 class="msg-listedit-header-label"></h1> + <h1 class="msg-listedit-header-label" data-l10n-id="message-multiedit-header" data-l10n-args='{ "n": "0" }'></h1> </header> </section> <!-- Search header --> diff --git a/apps/email/js/cards/message_list.js b/apps/email/js/cards/message_list.js index 7590de8..e6b46c0 100644 --- a/apps/email/js/cards/message_list.js +++ b/apps/email/js/cards/message_list.js @@ -481,10 +481,8 @@ MessageListCard.prototype = { selectedMessagesUpdated: function() { var headerNode = this.domNode.getElementsByClassName('msg-listedit-header-label')[0]; - headerNode.textContent = - mozL10n.get('message-multiedit-header', - { n: this.selectedMessages.length }); - + navigator.mozL10n.localize(headerNode, 'message-multiedit-header', + { n: this.selectedMessages.length }); var starBtn = this.domNode.getElementsByClassName('msg-star-btn')[0], Andrew, do you have better methods to solve all issues of language localization?

(In reply to Chaochao Huang from comment #26) > There are other issues of language localization. Yes, as I noted in comment 9, the fixes in the patch on this bug do not address all the l10n bugs in the app. However, those other bugs correct themselves when the app is restarted, whereas the problem on this bug was that we were (effectively) not relocalizing when the app restarted. Also noted in comment 9, unless things have improved on the Tarako front, the email app gets killed when it's in the background, so dynamic relocalization seems less likely to matter for Tarako. What device and version are you testing on? > 5. Back to Email, you will find inbox still in English. If you are referring to the folder name, "Inbox", then I believe this is bug 905869. The app needs to be restarted to update the set of localized folder names. > 7. You will find login interface still in English. I'm not sure what strings you're referring to here or what part of the account setup interface. Do you have a screenshot and/or can you elaborate? It's possible this fix isn't doing what it should be doing or it could be part of bug 906580 or it could be something else. (In reply to Chaochao Huang from comment #27) > Open the edit mode UI of email. The msg-listedit-header-label still is > 'edit' in English after language changing to French. It should be in French. > Close the edit mode UI of email, then back to the edit mode UI of email. The > msg-listedit-header-label displays normally. This is fixed on trunk/v2.1 by bug 906580.

> (In reply to Chaochao Huang from comment #26) > What device and version are you testing on? Gaia 3feb37ee2ed2319c9e556728723a5517dc1663ea Gecko c77f269933144005aeb8d98e3aa2738afd7b142e BuildID 20140801061635 Version 30.0 ro.build.version.incremental=334 ro.build.date=Fri Aug 1 06:13:36 CST 2014 > Do you have a screenshot and/or can you elaborate? There is a video about issues in attachments.

So the problem is that mozL10n.ready has flakey/bad semantics prior to bug 993188 landing on v2.0. So in v1.3t and v1.4 templates will not be re-localized, resulting in what you see in the video. In the interests of not dragging this out and since I think the authorization intent was clear, I've landed minimal fixes for v1.3t and v1.4 on the same authority: gaia/v1.3t: https://github.com/mozilla-b2g/gaia/pull/22451 https://github.com/mozilla-b2g/gaia/commit/51e7c900629781a956a1ca99186032c3f5d19ae2 gaia/v1.4: https://github.com/mozilla-b2g/gaia/pull/22452 https://github.com/mozilla-b2g/gaia/commit/01c7740a20d9863c3a7bf3dbecca7a83e07be695 Note that as discussed on comment 28 bug 905869 and bug 906580 continue to affect v1.3t/v1.4/v2.0 and should be discussed on those bugs, not this bug.