Closed Bug 1100808 Opened 8 years ago Closed 7 years ago

[Stingray] remove mozL10n.get and deprecated l10n API usage from tv_apps

Categories

(Firefox OS Graveyard :: Gaia, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: johnhu, Assigned: rohan1395)

References

Details

Attachments

(2 files)

The code in tv_apps contains deprecated l10n API usage. We should remove all of them.

Please read and follow this: https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/localization_code_best_practices

I will check and separate bugs for each app.
Attached file search for mozL10n.get
This is a list of mozL10n.get. All of them are in smart-system app. We may write a porting patch from /apps/system/ to /tv_apps/smart-system/
I had made another search on "using l10n-id on elements with child elements". There is no one violate it.
Commit to clean up the mozL10n.get is https://github.com/mozilla-b2g/gaia/commit/52ee3c33aacd17ba32de3e187ecfc554ef127993 . I think it's easy to make a port to tv_apps/smart-system.
The commit I mentioned at comment 3 is for apps/system.
Any progress with that?
Attached file Clean up mozl10n uses
Attachment #8542952 - Flags: review?(timdream)
Attachment #8542952 - Flags: review?(timdream) → review?(im)
Comment on attachment 8542952 [details] [review]
Clean up mozl10n uses

Rohan,

Thanks for the patch. This patch looks pretty good. It's pretty close to land. I remove the review flag because I found one syntax error and a wrong code in the patch. Please find my comments at pull request and put the review flag back once you finish it.
Attachment #8542952 - Flags: review?(im)
Assignee: nobody → rohan1395
Attachment #8542952 - Flags: review?(im)
Rohan,

Sorry for not telling you how to test TV build. Please use the following command to make a TV build for b2g desktop:

DEVICE_DEBUG=1 GAIA_DEVICE_TYPE=tv make

To run it at b2g-desktop you may use:

/Applications/B2G.app/Contents/MacOS/b2g-bin -profile /.../path/to/your/gaia/profile -screen 1920x1080

If you want to flash into your device, you may use the following:

DEVICE_DEBUG=1 GAIA_DEVICE_TYPE=tv make reset-gaia

Be aware that we use 1080p as default resolution. You might get a shrinking UI if you install into your device.
Comment on attachment 8542952 [details] [review]
Clean up mozl10n uses

Rohan,

I still found a few syntax errors here. Please update it and put the review flag again. Thanks.
Attachment #8542952 - Flags: review?(im)
Comment on attachment 8542952 [details] [review]
Clean up mozl10n uses

>https://github.com/mozilla-b2g/gaia/pull/27079
Attachment #8542952 - Flags: review?(im)
Comment on attachment 8542952 [details] [review]
Clean up mozl10n uses

Rohan,

Thanks for this great patch!!
Attachment #8542952 - Flags: review?(im) → review+
Rohan, I can merge your patch but you need to resolve the conflict against the current master.
Flags: needinfo?(rohan1395)
Zibi, the conflicts have been resolved.
Flags: needinfo?(rohan1395) → needinfo?(gandalf)
Thanks! I'll land this once the Gaia is open.
Flags: needinfo?(gandalf)
Rohan, there are still some mozl10n.get that can be cleaned up in tv_apps.

Do you want to work on the follow-up patch? If you don't think you will have time, I can try to tackle that.

Some notes:

1) https://github.com/mozilla-b2g/gaia/blob/master/tv_apps/app-deck/js/context_menu.js#L77-L84

This is fortunately not true.

You just need to set:

pin-to-home=Pin to Home
pin-to-home.label=Pin to Home

in app-deck.en-US.properties and it will be translated properly without mozL10n.get :)

2) there's a whole issue of Folder.name, which really should use Folder.nameL10n [0] and be resolved in UI only instead of storing a string.

John, would you prefer to have this refactor be a separate bug?

3) I would like to leave the tv_apps/smart-system/accessibility.js for now. We will want to make it async (possibly using mozL10n.formatValue), but I'd isolate it.

4) Same for smart-system/js/app_install_manager.js's humanizeSize

5) bluetooth seems to be using old NotificationHelper. It should be synced with the one from shared, which returns a Promise and support L10n API properly.

6) tv_apps/smart-system/js/captive_portal.js, tv_apps/smart-system/js/screenshot.js and tv_apps/smart-system/js/devtools/logshake.js should use the new NotificationHelper instead of raw W3C Notification.

7) Same for tv_apps/smart-system/js/external_storage_monitor.js

8) tv_apps/smart-system/js/ime_menu.js and tv_apps/smart-system/js/permission_manager.js should interpolate Template by setting data-l10n-id, instead of passing strings.



[0] https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/localization_code_best_practices#Writing_APIs_that_operate_on_L10nIDs
Flags: needinfo?(rohan1395)
Flags: needinfo?(im)
Hi zb,

I will file a bug and do it now. But I need to confirm that:
1. I just leave item 3 and 4 as it current status
2. Using new NotificationHelper in shared, item 5, 6, 7
3. change the template content to use data-l10n-id, item 8

Am I correct?

BTW, I don't understand the item 2, Folder.name, totally. May you give me more information?
Flags: needinfo?(im) → needinfo?(gandalf)
(In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #17)
> Hi zb,
> 
> I will file a bug and do it now. But I need to confirm that:
> 1. I just leave item 3 and 4 as it current status
> 2. Using new NotificationHelper in shared, item 5, 6, 7
> 3. change the template content to use data-l10n-id, item 8
> 
> Am I correct?

Yes!

> BTW, I don't understand the item 2, Folder.name, totally. May you give me
> more information?

Apologies for a late response. I responded in bug 1121816 comment 2. Let me know if that helps!
Flags: needinfo?(gandalf)
Hey guys,

Thanks for being active with updating the uses of L10n API!

There are still 24 references of mozL10n.get in tv_apps.

I identified some items that are isolated enought:

 - Folder.name should be L10n type object (so, {raw: "string"} and/or {id: "l10nId"} type) and only resolved when displaying
 - AppModalDialog.getTitle and getMessage should return L10n type object and apply it using mozL10n.setAttributes on the node instead of innerHTML.
 - and of course bug 1121814 is the biggest one since it will move all Notifications
TV doesn't have any mozL10n.get calls anymore!
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Flags: needinfo?(rohan1395)
You need to log in before you can comment on or make changes to this bug.