[MD settings] adding load time data behavior for i18n strings

[MD settings] adding load time data behavior for i18n strings

This is a step toward adding this behavior to CL 1388353003.

BUG=425627
Committed: https://crrev.com/2aee820e2030bb54441910e3dc668a44598c5417
Cr-Commit-Position: refs/heads/master@{#353174}

[dschuyler, 2015-10-08 19:39:08]

dschuyler@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2015-10-08 19:52:01]

dbeam@chromium.org changed reviewers:
+ stevenjb@chromium.org

[Dan Beam, 2015-10-08 19:52:02]

lgtm but wait for stevenjb@'s opinion on this matter

[stevenjb, 2015-10-08 19:59:35]

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js
(right):

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js:18:
var LoadTimeDataI18nBehavior = {
This strikes me as unnecessarily lengthy. I just named it i18nBehavior (and
i19n_behavior.js). imho, "load_time_data" is an implementation detail that might
change. We also may want to add additional methods to this behavior that are not
associated with loadTimeData.

Dan, thoughts on that?

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js:21:
* @param {...var_args} Placeholders required by the string ($1-9).
Ah, good, I was just investigating how to correctly specify this. I assume this
compiles correctly if we call i18n(id) with no args?

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js:25:
return loadTimeData.getStringF.apply(loadTimeData, arguments);
Is 'arguments' some magical JS thing, or should it be s/arguments/id, var_args/
?

We should definitely have a compuled_resources.gyp for this.

Also, we're going to rely on this enough that it's probably test-worthy...

[Dan Beam, 2015-10-08 20:48:59]

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js
(right):

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js:18:
var LoadTimeDataI18nBehavior = {
On 2015/10/08 19:59:34, stevenjb wrote:
> This strikes me as unnecessarily lengthy. I just named it i18nBehavior (and
> i19n_behavior.js). imho, "load_time_data" is an implementation detail that
might
> change. We also may want to add additional methods to this behavior that are
not
> associated with loadTimeData.
> 
> Dan, thoughts on that?

that's fine, the clients just need to understand which ID to pass (which is
currently tied to loadTimeData)

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js:21:
* @param {...var_args} Placeholders required by the string ($1-9).
On 2015/10/08 19:59:34, stevenjb wrote:
> Ah, good, I was just investigating how to correctly specify this. I assume
this
> compiles correctly if we call i18n(id) with no args?

yes

https://developers.google.com/closure/compiler/docs/js-for-compiler?hl=en

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js:25:
return loadTimeData.getStringF.apply(loadTimeData, arguments);
On 2015/10/08 19:59:34, stevenjb wrote:
> Is 'arguments' some magical JS thing, or should it be s/arguments/id,
var_args/
> ?

no, no

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/a...
(first google result for "javascript arguments")
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...

> 
> We should definitely have a compuled_resources.gyp for this.
> 
> Also, we're going to rely on this enough that it's probably test-worthy...

sure

[stevenjb, 2015-10-08 20:54:49]

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js
(right):

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js:25:
return loadTimeData.getStringF.apply(loadTimeData, arguments);
On 2015/10/08 20:48:59, Dan Beam wrote:
> On 2015/10/08 19:59:34, stevenjb wrote:
> > Is 'arguments' some magical JS thing, or should it be s/arguments/id,
> var_args/
> > ?
> 
> no, no
> 
>
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/a...
> (first google result for "javascript arguments")
>
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...
> 
> > 
> > We should definitely have a compuled_resources.gyp for this.
> > 
> > Also, we're going to rely on this enough that it's probably test-worthy...
> 
> sure

To be clear: I don't think we need to necessarily hold this CL up on tests, but
it would be a good TODO item.

[dschuyler, 2015-10-08 21:17:05]

PTAL, this has the shortened names (i18n_behavior).

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js
(right):

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js:21:
* @param {...var_args} Placeholders required by the string ($1-9).
On 2015/10/08 19:59:34, stevenjb wrote:
> Ah, good, I was just investigating how to correctly specify this. I assume
this
> compiles correctly if we call i18n(id) with no args?

I do have a i18n('id') in my test, and the closure compile did not complain.

https://codereview.chromium.org/1391333005/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/load_time_data_i18n_behavior/load_time_data_i18n_behavior.js:25:
return loadTimeData.getStringF.apply(loadTimeData, arguments);
On 2015/10/08 19:59:34, stevenjb wrote:
> Is 'arguments' some magical JS thing, or should it be s/arguments/id,
var_args/
> ?
Yeah, 'arguments' is some magical JS thing.

> 
> We should definitely have a compuled_resources.gyp for this.
> 
> Also, we're going to rely on this enough that it's probably test-worthy...

[stevenjb, 2015-10-08 23:23:46]

lgtm

[Dan Beam, 2015-10-08 23:35:27]

still lgtm

[dschuyler, 2015-10-08 23:37:11]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2015-10-08 23:39:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1391333005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1391333005/20001

[commit-bot: I haz the power, 2015-10-08 23:49:11]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-08 23:50:37]

Patchset 2 (id:??) landed as
https://crrev.com/2aee820e2030bb54441910e3dc668a44598c5417
Cr-Commit-Position: refs/heads/master@{#353174}