Move responsibility for l10n parameters to the caller.

Move responsibility for l10n parameters to the caller.

BUG=132370
TEST=No behavioural change expected.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=148005

[Jamie, 2012-06-26 17:34:35]

ptal

[Sergey Ulanov, 2012-06-28 01:11:24]

https://chromiumcodereview.appspot.com/10661058/diff/1/remoting/host/plugin/h...
File remoting/host/plugin/host_script_object.cc (right):

https://chromiumcodereview.appspot.com/10661058/diff/1/remoting/host/plugin/h...
remoting/host/plugin/host_script_object.cc:1079: plugin_, localize_func,
args.get(), number_of_substitutions + 1,
Actually chrome.i18n.getMessage() takes only two parameters - the second
parameter is either one string or an array of strings, so this works correctly
only for the case when there is only one substitution.

[Jamie, 2012-06-28 23:02:41]

https://chromiumcodereview.appspot.com/10661058/diff/1/remoting/host/plugin/h...
File remoting/host/plugin/host_script_object.cc (right):

https://chromiumcodereview.appspot.com/10661058/diff/1/remoting/host/plugin/h...
remoting/host/plugin/host_script_object.cc:1079: plugin_, localize_func,
args.get(), number_of_substitutions + 1,
On 2012/06/28 01:11:24, sergeyu wrote:
> Actually chrome.i18n.getMessage() takes only two parameters - the second
> parameter is either one string or an array of strings, so this works correctly
> only for the case when there is only one substitution.

So it does--good catch! Passing arrays over NPAPI is quite painful, so I've
opted to support only one substitution for now. A relatively simple alternative
would be to pass the strings as individual parameters and turn them into an
array in Javascript, but I don't think that's worth doing right now.

[Sergey Ulanov, 2012-06-29 20:11:38]

lgtm when my nits are addressed

https://chromiumcodereview.appspot.com/10661058/diff/13001/remoting/host/plug...
File remoting/host/plugin/host_script_object.cc (right):

https://chromiumcodereview.appspot.com/10661058/diff/13001/remoting/host/plug...
remoting/host/plugin/host_script_object.cc:1055:
/*i18n-content*/"MESSAGE_SHARED", "$1",
nit: incorrect indentation.

https://chromiumcodereview.appspot.com/10661058/diff/13001/remoting/host/plug...
remoting/host/plugin/host_script_object.cc:1068: NPObject* localize_func, const
char* tag, const char* substitution,
nit: move each argument to a separate line

https://chromiumcodereview.appspot.com/10661058/diff/13001/remoting/host/plug...
remoting/host/plugin/host_script_object.cc:1073: if (substitution) {
nit: don't need {} here

[Jamie, 2012-06-29 20:22:20]

fyi

https://chromiumcodereview.appspot.com/10661058/diff/13001/remoting/host/plug...
File remoting/host/plugin/host_script_object.cc (right):

https://chromiumcodereview.appspot.com/10661058/diff/13001/remoting/host/plug...
remoting/host/plugin/host_script_object.cc:1055:
/*i18n-content*/"MESSAGE_SHARED", "$1",
On 2012/06/29 20:11:38, sergeyu wrote:
> nit: incorrect indentation.

Done.

https://chromiumcodereview.appspot.com/10661058/diff/13001/remoting/host/plug...
remoting/host/plugin/host_script_object.cc:1068: NPObject* localize_func, const
char* tag, const char* substitution,
On 2012/06/29 20:11:38, sergeyu wrote:
> nit: move each argument to a separate line

Done.

https://chromiumcodereview.appspot.com/10661058/diff/13001/remoting/host/plug...
remoting/host/plugin/host_script_object.cc:1073: if (substitution) {
On 2012/06/29 20:11:38, sergeyu wrote:
> nit: don't need {} here

We're inconsistent wrt this, even within this file, but braces for one-line
control blocks are a slight majority.

[Sergey Ulanov, 2012-07-23 22:19:33]

Is this CL blocked on anything?

[commit-bot: I haz the power, 2012-07-23 22:26:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/10661058/8003

[Jamie, 2012-07-23 22:27:58]

On 2012/07/23 22:19:33, sergeyu wrote:
> Is this CL blocked on anything?

I think I must have believed I'd already submitted it (hence the FYI, rather
than PTAL). I'll try the CQ; if that doesn't work it'll have to wait another few
days at least until I have time to rebase it.

[commit-bot: I haz the power, 2012-07-24 00:37:50]

Change committed as 148005