Last Comment Bug 1020136 - Clean up use cases of mozL10n.translate
: Clean up use cases of mozL10n.translate
Status: RESOLVED FIXED
:
Product: Firefox OS
Classification: Client Software
Component: Gaia::L10n (show other bugs)
: unspecified
: x86 All
P3 normal (vote)
: ---
Assigned To: Zibi Braniecki [:gandalf][:zibi]
:
:
Mentors:
Depends on: 992473 1041252 1041403 1047344 1047347 1052136 1057795
Blocks: 1075273
  Show dependency treegraph
 
Reported: 2014-06-04 00:33 PDT by Zibi Braniecki [:gandalf][:zibi]
Modified: 2014-10-02 14:50 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
pull request (46 bytes, text/x-github-pull-request)
2014-09-30 18:01 PDT, Zibi Braniecki [:gandalf][:zibi]
stas: review+
Details | Review | Splinter Review
new pull request (46 bytes, text/x-github-pull-request)
2014-10-02 09:53 PDT, Zibi Braniecki [:gandalf][:zibi]
gandalf: review+
Details | Review | Splinter Review

User Story
Probably all use cases of mozL10n.translate can be safely removed since in master mozL10n.translate is no-op anyway.

If any of those cases needs manual DOM translation, use mozL10n.translateFragment(node) instead.      
Description User image Zibi Braniecki [:gandalf][:zibi] 2014-06-04 00:33:24 PDT
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 User image Mike Hoye [:mhoye] 2014-06-11 09:40:31 PDT
Gandalf, can I add you as a mentor to this bug?
Comment 2 User image Zibi Braniecki [:gandalf][:zibi] 2014-06-11 09:48:42 PDT
yes!
Comment 3 User image Paul Z 2014-06-19 14:54:33 PDT
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 User image Zibi Braniecki [:gandalf][:zibi] 2014-06-19 16:27:07 PDT
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 User image Paul Z 2014-06-20 03:53:14 PDT
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 User image Zibi Braniecki [:gandalf][:zibi] 2014-06-20 08:17:27 PDT
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 User image Zibi Braniecki [:gandalf][:zibi] 2014-07-09 12:42:08 PDT
Paul, any progress on that?
Comment 8 User image Paul Z 2014-07-09 15:20:35 PDT
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 User image Paul Z 2014-07-19 06:34:01 PDT
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 User image Mike Hoye [:mhoye] 2014-07-19 10:37:55 PDT
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 User image Zibi Braniecki [:gandalf][:zibi] 2014-07-19 15:45:13 PDT
Sure Paul, hope you'll find something for yourself :) Thanks for giving it a try!
Comment 12 User image Zibi Braniecki [:gandalf][:zibi] 2014-09-30 18:01:24 PDT
Created attachment 8497890 [details] [review]
pull request

Thanks to a lot of work by many people, we're down to 7 uses. This PR removes them.
Comment 13 User image Staś Małolepszy :stas 2014-10-02 07:01:40 PDT
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 User image Zibi Braniecki [:gandalf][:zibi] 2014-10-02 09:52:41 PDT
Comment on attachment 8497890 [details] [review]
pull request

>https://github.com/mozilla-b2g/gaia/pull/24713
Comment 15 User image Zibi Braniecki [:gandalf][:zibi] 2014-10-02 09:53:22 PDT
Created attachment 8498968 [details] [review]
new pull request

new pull request, carrying over r=stas
Comment 16 User image Zibi Braniecki [:gandalf][:zibi] 2014-10-02 14:50:47 PDT
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 :)

Note You need to log in before you can comment on or make changes to this bug.