The default bug view has changed. See this FAQ.

Clean up use cases of mozL10n.translate

RESOLVED FIXED

Status

Firefox OS
Gaia::L10n
P3
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: gandalf, Assigned: gandalf)

Tracking

Firefox Tracking Flags

(Not tracked)

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 attachment, 1 obsolete attachment)

46 bytes, text/x-github-pull-request
gandalf
: review+
Details | Review | Splinter Review
(Assignee)

Description

3 years ago
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

3 years ago
Depends on: 992473
(Assignee)

Updated

3 years ago
Priority: -- → P3
(Assignee)

Updated

3 years ago
Whiteboard: [good first bug]

Comment 1

3 years ago
Gandalf, can I add you as a mentor to this bug?
Flags: needinfo?(gandalf)
(Assignee)

Comment 2

3 years ago
yes!
Flags: needinfo?(gandalf)

Updated

3 years ago
Whiteboard: [good first bug] → [good first bug][mentor=gandalf]
Mentor: gandalf@aviary.pl
Whiteboard: [good first bug][mentor=gandalf] → [good first bug]

Comment 3

3 years ago
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

3 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)

Comment 5

3 years ago
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

3 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.
(Assignee)

Comment 7

3 years ago
Paul, any progress on that?
Flags: needinfo?(pzwart)

Comment 8

3 years ago
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)

Comment 9

3 years ago
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

3 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

3 years ago
Sure Paul, hope you'll find something for yourself :) Thanks for giving it a try!
(Assignee)

Updated

3 years ago
Depends on: 1041252
(Assignee)

Updated

3 years ago
Depends on: 1041403
(Assignee)

Updated

3 years ago
User Story: (updated)
(Assignee)

Updated

3 years ago
Depends on: 1047344
(Assignee)

Updated

3 years ago
Depends on: 1047347
(Assignee)

Updated

3 years ago
Depends on: 1052136
(Assignee)

Updated

3 years ago
Depends on: 1057795
(Assignee)

Comment 12

3 years ago
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.
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
Mentor: gandalf@aviary.pl
Whiteboard: [good first bug]
(Assignee)

Updated

3 years ago
Blocks: 1075273
(Assignee)

Updated

3 years ago
Attachment #8497890 - Flags: review?(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.
Attachment #8497890 - Flags: review?(stas) → review+
(Assignee)

Comment 14

3 years ago
Comment on attachment 8497890 [details] [review]
pull request

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

Comment 15

3 years ago
Created attachment 8498968 [details] [review]
new pull request

new pull request, carrying over r=stas
Attachment #8498968 - Flags: review+
(Assignee)

Updated

3 years ago
Attachment #8497890 - Attachment is obsolete: true
(Assignee)

Comment 16

3 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
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.