1062749 - [Dialer] Make prettyDuration use l10n attributes instead of setting textContent. Follow up bug 1061120

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[David Garcia [:davidg][retired]]

Currently prettyDuration (shared/js/dialer/utils.js) is used like this:

someHTMLElement.textContent = Utils.prettyDuration(/* ... */);

So, it doesn't respond to changes in the locales. Change it to something like:

prettyDuration: function(node, duration, l10nIdPrefix) {
  var l10nId = l10nIdPrefix + /* do the body of the function */;
  node.dataset.l10nId = l10nId;
  node.dataset.l10nArgs = JSON.stringify(durationL10n);
}

Comment 1

[Doug Sherk (:drs) (inactive)]

I talked with Zibi and he says that they're gradually moving away the shared l10n libraries such as l10n_date.js from the |node.textContent = library.function()| pattern and more towards passing a node in as in comment 0. They'll be introducing helpers to do this. They haven't done this yet because there are so many callsites for these functions. Also, when we're calling DateTimeFormat, this isn't an option. We have to use mozL10n.get() in this case.

In our case, prettyDuration only has 2 callsites, both of which are of the form |node.textContent = Utils.prettyDuration()| so this seems like something obvious we can do.

One thing Zibi pointed out is that we can use the setAttributes helper on the mozL10n web API instead of setting each attribute ourselves, like so:

navigator.mozL10n.setAttributes(node, l10nId, JSON.stringify(durationL10n));

Comment 2

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

> navigator.mozL10n.setAttributes(node, l10nId, JSON.stringify(durationL10n));

Actually, navigator.mozL10n.setAttributes(node, l10nId, durationL10n); - the helper is there exactly to hide the JSON.parse/stringify (there's also mozL10n.getAttributes for ya all!)

Comment 3

[David Garcia [:davidg][retired]]

Attachment: patch

Comment 4

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8486256 [details] [review]
patch

There's a few small issues with this so I'd like to see another version. But I think it'll be ready after that. See my comments on the PR.

Comment 5

[David Garcia [:davidg][retired]]

Comment on attachment 8486256 [details] [review]
patch

second round :)

Comment 6

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8486256 [details] [review]
patch

One tiny nit and then we're good to go. I still have to talk with Zibi about the mozL10n.once() call, so I'll land this later if he gives the go-ahead.

Comment 7

[David Garcia [:davidg][retired]]

Done.
Would you mind landing it Doug? Thanks!

Comment 8

[Doug Sherk (:drs) (inactive)]

https://github.com/mozilla-b2g/gaia/commit/fe416cc9b2128c3b817134f527233a8bedf99b08