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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[John Hu [:johnhu][:johu][:醬糊小弟]]

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.

Comment 1

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: 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/

Comment 2

[John Hu [:johnhu][:johu][:醬糊小弟]]

I had made another search on "using l10n-id on elements with child elements". There is no one violate it.

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

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.

Comment 4

[John Hu [:johnhu][:johu][:醬糊小弟]]

The commit I mentioned at comment 3 is for apps/system.

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

Any progress with that?

Comment 6

[Rohan Rawat]

Attachment: Clean up mozl10n uses

Comment 7

[John Hu [:johnhu][:johu][:醬糊小弟]]

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.

Comment 8

[John Hu [:johnhu][:johu][:醬糊小弟]]

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 9

[John Hu [:johnhu][:johu][:醬糊小弟]]

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.

Comment 10

[Rohan Rawat]

Comment on attachment 8542952 [details] [review]
Clean up mozl10n uses

>https://github.com/mozilla-b2g/gaia/pull/27079

Comment 11

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8542952 [details] [review]
Clean up mozl10n uses

Rohan,

Thanks for this great patch!!

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

Rohan, I can merge your patch but you need to resolve the conflict against the current master.

Comment 13

[Rohan Rawat]

Zibi, the conflicts have been resolved.

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Thanks! I'll land this once the Gaia is open.

Comment 15

[Zibi Braniecki [:zbraniecki][: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

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

Commit: https://github.com/mozilla-b2g/gaia/commit/cef35c2f739bd760a8f76eb5e825305469b39317
Merge: https://github.com/mozilla-b2g/gaia/commit/3feda3792451ece27977a00dc3b24731be1e6beb

Comment 17

[John Hu [:johnhu][:johu][:醬糊小弟]]

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?

Comment 18

[Zibi Braniecki [:zbraniecki][: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!

Comment 19

[Zibi Braniecki [:zbraniecki][: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

Comment 20

[Zibi Braniecki [:zbraniecki][:gandalf]]

TV doesn't have any mozL10n.get calls anymore!