1121816 - [Stingray][smart-system] Remove mozL10n.get in context menu

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::TV::System, defect)

Description

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

+++ This bug was initially created as a clone of Bug #1100808 +++

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.

Comment 1

[Tzu-Lin Huang [:dwi2][:tzhuang]]

(In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #0)

> 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.
I think we should leave this to bug 1115642.

Comment 2

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

(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #1)
> (In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #0)
> 
> > 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.
> I think we should leave this to bug 1115642.

Possibly. My recommendation is more focused on the l10n API aspect of that: https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/localization_code_best_practices#Writing_APIs_that_operate_on_L10nIDs

In particular, if you have a Folder name that may be localizable, you should store its l10nId, not its localized string.

So, instead of:

class Folder {
  name: "My Name",
}

it should be:

class Folder {
  name: "l10nId"
}

and following the convention from the document I linked it can be:

name: {id: "l10nId"}
name: {id: "l10nId", args: {}}
name: {raw: "User provided, not translatable name"}

and then you should resolve it at the time when you are drawing:

var folderTitleNode = docment.createElement('div');
navigator.mozL10n.setAttributes(folderTitleNode, Folder.name.id, Folder.name.args);

Storing a localizable string in a data structure is an antipattern.

If you think it makes sense to move it to that bug, I'll be happy to provide guidance on how to fix that.

Comment 3

[Tzu-Lin Huang [:dwi2][:tzhuang]]

> If you think it makes sense to move it to that bug, I'll be happy to provide
> guidance on how to fix that.
Yes, I'll like to focus this bug on fixing context menu part, and to fix localized card name (that includes folder) in bug 1115642.

Thanks a lot for your help.

Comment 4

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

Sounds good to me!

Comment 5

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Attachment: pull request

Hi Zibi,

Could you help to review the patch? thanks

John, because this may also affect system browser, please provide some feedback on my patch. Thanks

Comment 6

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

Comment on attachment 8550180 [details] [review]
pull request

lgtm!

You could follow the API pattern from [0] just in case you will want to at some point enable also {id: , args: } format.

In that case, instead of having objects with labelL10n or label, you could have label be always of a form:

"l10nId" or
{raw: "Raw string"}

for now, and later maybe you will want to add a third form:
{id: "l10nId", args: {}}

In that case what you currently assign to .label, would be .label = {raw: str};

and in the DOM building piece you just do:

if (obj.label.raw) {
  node.textContent = obj.label.raw;
} else {
  node.setAttribute('data-l10n-id', obj.label);
}

But that's totally optional. You can land it as-is.

[0] https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/localization_code_best_practices#Writing_APIs_that_operate_on_L10nIDs

Comment 7

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

Comment on attachment 8550180 [details] [review]
pull request

looks good to me.

Comment 8

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Thanks, I filed bug 1123184 to do the work of enabling {id: , args: } format.

I'll land it once gaia-try is green.

Also I change the component because we have Gaia:TV:System now.

Comment 9

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Gaia try is green 
https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=928663074796

Landed on master 
https://github.com/mozilla-b2g/gaia/commit/d8bcb2c253f3e19904ca6239daf9bd012e5642f7