Closed Bug 1020136 Opened 10 years ago Closed 10 years ago

Clean up use cases of mozL10n.translate

Categories

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

x86
All
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

Details

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.

Attachments

(1 file, 1 obsolete file)

(deleted), text/x-github-pull-request
zbraniecki
: review+
Details
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.
Depends on: 992473
Priority: -- → P3
Whiteboard: [good first bug]
Gandalf, can I add you as a mentor to this bug?
Flags: needinfo?(gandalf)
yes!
Flags: needinfo?(gandalf)
Whiteboard: [good first bug] → [good first bug][mentor=gandalf]
Mentor: gandalf
Whiteboard: [good first bug][mentor=gandalf] → [good first bug]
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?
Flags: needinfo?(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.
Flags: needinfo?(gandalf)
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?)
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.
Paul, any progress on that?
Flags: needinfo?(pzwart)
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.
Flags: needinfo?(pzwart)
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!
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.
Sure Paul, hope you'll find something for yourself :) Thanks for giving it a try!
Depends on: 1041252
Depends on: 1041403
User Story: (updated)
Depends on: 1047344
Depends on: 1047347
Depends on: 1052136
Depends on: 1057795
Attached file pull request (obsolete) (deleted) —
Thanks to a lot of work by many people, we're down to 7 uses. This PR removes them.
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Mentor: gandalf
Whiteboard: [good first bug]
Blocks: 1075273
Attachment #8497890 - Flags: review?(stas)
Comment on attachment 8497890 [details] 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.
Attachment #8497890 - Flags: review?(stas) → review+
Attached file new pull request (deleted) —
new pull request, carrying over r=stas
Attachment #8498968 - Flags: review+
Attachment #8497890 - Attachment is obsolete: true
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 :)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: