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)
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)
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.
Assignee | ||
Updated•10 years ago
|
Priority: -- → P3
Assignee | ||
Updated•10 years ago
|
Whiteboard: [good first bug]
Updated•10 years ago
|
Whiteboard: [good first bug] → [good first bug][mentor=gandalf]
Updated•10 years ago
|
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)
Assignee | ||
Comment 4•10 years ago
|
||
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?)
Assignee | ||
Comment 6•10 years ago
|
||
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.
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!
Comment 10•10 years ago
|
||
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.
Assignee | ||
Comment 11•10 years ago
|
||
Sure Paul, hope you'll find something for yourself :) Thanks for giving it a try!
Assignee | ||
Updated•10 years ago
|
User Story: (updated)
Assignee | ||
Comment 12•10 years ago
|
||
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
Assignee | ||
Updated•10 years ago
|
Mentor: gandalf
Whiteboard: [good first bug]
Assignee | ||
Updated•10 years ago
|
Attachment #8497890 -
Flags: review?(stas)
Comment 13•10 years ago
|
||
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+
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8497890 [details]
pull request
>https://github.com/mozilla-b2g/gaia/pull/24713
Assignee | ||
Comment 15•10 years ago
|
||
new pull request, carrying over r=stas
Attachment #8498968 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Attachment #8497890 -
Attachment is obsolete: true
Assignee | ||
Comment 16•10 years ago
|
||
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.
Description
•