1020136 - Clean up use cases of mozL10n.translate

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::L10n, defect, P3)

Description

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

After landing bug 992473, we will have a lot of use cases of mozL10n.translate which will be obsolete and should be removed.

There are 37 uses of mozL10n.translate in apps. I'm fairly confident that all of them should be removed.

Comment 1

[Mike Hoye [:mhoye]]

Gandalf, can I add you as a mentor to this bug?

Comment 2

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

yes!

Comment 3

[Paul Z]

I like to try this one as 'my-first-bug'. I found around 43 calls to mozL10n.translate() in the B2G/gaia dir (and one in gecko dir) with a grep. Should they be replaced with something else? Or just removed as it says in the discription of the bug?
I found the calls in the following apps: clock, ringtones, costcontrol, communications, pdfjs, calendar, keyboard, camera, system, settings, browser, sms, sharedtest, homescreen and email. I guess these changes should not be in one big patch?

Comment 4

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

Hi Paul!

That's awesome that you want to help us with this. I'll be very happy to mentor you!

Let me draw some background:

Two days ago we landed bug 992473. It basically makes us react to Node changes automatically making mozL10n.translate obsolete.

Unfortunately, there are corner cases where we still need it. Two of them are localization of iframe content (bug 1020130) and shadow DOM content (bug 1026236). 

There are other corner cases, like the one in email, where they want to cache localized DOM before they inject it into the document.

We want to let the MutationObservers patch bake in for a week or two before we start removing those calls, and then we need to go through them one by one making sure that they're safe to be removed.

My recommendation is:
 - pick one app that uses translate
 - get to understand why it is there. If it's use cases has been rendered obsolete by MutationObserver, proceed to remove it. If not, switch it to mozL10n.translateFragment
 - test if the MO properly triggers and translates the content without mozL10n.translate
 - write a patch
 - attach it here
 - we'll ask for review from someone from that apps' team
 - proceed to the next app


It may sound like a lot, but it'll actually be fairly easy and will give you a good insight into how those apps do their front-end JS code. I know that, because while working on bug 992473, I learned a lot :)

Let me know if you have more questions!
I'm also on IRC #gaia and #l20n if you need a quick help.

Comment 5

[Paul Z]

Thanks for the info. I'm actually quite excited to help out but will need some time 'starting up'. I'll pick one app and start there. How do I exactly test my changes? I got the arm-emulator running and ran the Marionette tests. Or are there simpler ways? (Like the Fx OS Desktop?)

Comment 6

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

The easiest way to test changes is to go for that: https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia

This will give you a way to run Firefox OS in your Firefox desktop which gives you nice access to all dev tools.

There are other ways (you can download B2G Desktop from nightly.mozilla.org and launch gaia against it instead of launching against Firefox) but this one will be a good first step.

Comment 7

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

Paul, any progress on that?

Comment 8

[Paul Z]

Not yet. I have been ill last week and im still recovering. I'm going to do my best this week to get things running. Lets set a deadline for progress on Wednesday the 16th.

Comment 9

[Paul Z]

Okay, I tried but this one is too much for my current set of skills. It's available for others to take. Gandalf: thanks for your help!

Comment 10

[Mike Hoye [:mhoye]]

Thanks for taking a run at that, Paul. If you can email me, I'd like to help you find a bug that might be a better fit.

Comment 11

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

Sure Paul, hope you'll find something for yourself :) Thanks for giving it a try!

Comment 12

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

Attachment: pull request

Thanks to a lot of work by many people, we're down to 7 uses. This PR removes them.

Comment 13

[Staś Małolepszy :stas]

Comment on attachment 8497890 [details] [review]
pull request

You'll probably need to rebase and retrigger TBPL again and see if the Gij failure persists.  r=me with a green build.

Comment 14

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

Comment on attachment 8497890 [details] [review]
pull request

>https://github.com/mozilla-b2g/gaia/pull/24713

Comment 15

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

Attachment: new pull request

new pull request, carrying over r=stas

Comment 16

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

Yeah, got green Gij on the third try. Gaia-try is tricky ;)

Commit: https://github.com/zbraniecki/gaia/commit/ac5e02876e11ac65bce15c07815841d302dc9c73
Merge: https://github.com/mozilla-b2g/gaia/commit/9d190bbb1c1b6ea2128e3d2562a5f435150a137c

Closing this bug!
Thanks everyone for help :)