We don't use a lot of mozL10n.get in the SMS app; the rare places we use it, I think we can't do otherwise (eg: notification API, window.confirm, window.alert). But I'd be happy to be wrong :)

Let's see what Giovanni will find! :)

Julien is correct, there was not much that could be done. Much of the localisation is for dates - which I will not be focusing on - or alerts/confirmations. The main thing blocking progress for this bug is notifications, they are not localised - see Bug 967925. That is something I am looking into now.

Comment on attachment 8458830 [details] Some chnages to use node.setAttribute over mozL10n.get That looks good from l10n standpoint. let's get julien's r.

Comment on attachment 8458830 [details] Some chnages to use node.setAttribute over mozL10n.get I'm fine with the changes for action_menu.js, but in information.js we miss one important case. Added more comments on github. Thanks !

Gandalf, can you comment on the RTL discussion here? I'd rather want the simpler solution but I fail to see if this messes up RTL. I think it doesn't but Guivanni thinks it does :) You can find the discussion in the 3rd "outdated diff discussion". Thanks !

Ahmed, can you look at https://github.com/mozilla-b2g/gaia/pull/21941#discussion_r15298765 ?

Can we move forward without help from Ahmed? Flod?

Apologies for late reply I don't see anything suspicious with the current version of the patch. But just to note, AFAIK this: > sim-detail = «end of list» {{detail-string}} {{sim}} SIM «start of list» Doesn't happen at anyway because in RTL, Gecko will take care of reordering the context (and in the case of mixed RTL and LTR text this will be wrong too). So if we originally have > sim-detail = Sim {{sim}} {{detail-string}} All we usually do is replace the word Sim with it's equivalent in the desired RTL language and that's enough. This seems a little vague but that's what really happens, let's not forget that {{sim}} and {{detail-string}} will also be replaced by their correspondent text in the RTL locale so the final result is a complete RTL string, and because of that, changing the order of {{sim}} and {{detail-string}} will just wrongly reverse it. Hope this is helpful and not making it more confusing.. :)

(clearing NI, also considering I'm just starting to get a grasp of RTL) Only request: please explain in localization comments what is going on with all those variables.

Thanks Ahmed, so this is what I thought too. Giovanni, then I think we can use the simpler code :)

Comment on attachment 8458830 [details] Some chnages to use node.setAttribute over mozL10n.get let's do the other approach then ! Thanks for your patience

Comment on attachment 8458830 [details] Some chnages to use node.setAttribute over mozL10n.get one issue and some nits (I actually couldn't try the SIM part on a device as I have no dual SIM device on central right now -- I'm doing one now so it should be ok tomorrow). Thanks !

Giovanni, please, squash your commits into one, and rebase on top of the current master. It should help with the tests :)

Comment on attachment 8458830 [details] Some chnages to use node.setAttribute over mozL10n.get Redirecting the review to Oleg, to check that the SIM information display correctly in the report panel on a dual SIM device (as I just broke my Flame...)

Comment on attachment 8458830 [details] Some chnages to use node.setAttribute over mozL10n.get (In reply to Julien Wajsberg [:julienw] from comment #15) > Comment on attachment 8458830 [details] > Some chnages to use node.setAttribute over mozL10n.get > > Redirecting the review to Oleg, to check that the SIM information display > correctly in the report panel on a dual SIM device (as I just broke my > Flame...) SIM indicator in the report panel looks fine to me (basically as it was before + auto translation on language switch). So r+ for the report panel functional part.

Comment on attachment 8458830 [details] Some chnages to use node.setAttribute over mozL10n.get Just put the r- to fix the remaining indentation issues. Also please squash and add "r=gandalf,azasypkin,julien" in your commit log

Julien, I think we have everything ready for landing here. The try build is green except of one timed out test (I restarted the test).

Comment on attachment 8458830 [details] Some chnages to use node.setAttribute over mozL10n.get I'm sorry but there are still indentation issues :(

master: c7f8938522a18454809fb6ea0fd3eddef10a73ea

Comment on attachment 8458830 [details] Some chnages to use node.setAttribute over mozL10n.get r=me thanks a lot for your patience !

Hi Julienw Please uplift these to v2.0 as well. Thanks!

(In reply to ankit93040 from comment #23) > Hi Julienw > Please uplift these to v2.0 as well. Please don't ;-) New string in this patch, and I think we don't move these patches to 2.0, since it has a different l10n.js

A couple more notes: * please add comments when adding non obvious variables (i.e. "detailString" in this case) * is this hard-coded separator wanted? https://github.com/giovannic/gaia/blob/ea9a3d91c564b2a0816106318477013edc721f17/apps/sms/js/information.js#L113

(In reply to Francesco Lodolo [:flod] from comment #25) > A couple more notes: > * please add comments when adding non obvious variables (i.e. "detailString" > in this case) right, sorry :( > * is this hard-coded separator wanted? > https://github.com/giovannic/gaia/blob/ > ea9a3d91c564b2a0816106318477013edc721f17/apps/sms/js/information.js#L113 Yeah, it's been discussed with Pike AFAIK.