Add unit test for l10n.js

remoting/webapp/unittests/l10n_unittest.js

Index: remoting/webapp/unittests/l10n_unittest.js
diff --git a/remoting/webapp/unittests/l10n_unittest.js b/remoting/webapp/unittests/l10n_unittest.js
new file mode 100644
index 0000000000000000000000000000000000000000..16018f20915b12f8fa540a77aa766f7fbe50ada7
--- /dev/null
+++ b/remoting/webapp/unittests/l10n_unittest.js
@@ -0,0 +1,106 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+(function() {
+
+'use strict';
+
+module('l10n', {
+  setup: function() {
+    chrome.i18n = chrome.i18n || {};
+    chrome.i18n.getMessage = chrome.i18n.getMessage || function() {};

[Jamie, 2014/07/18 02:04:50]

The use of || here doesn't feel right. It's generally used in places where you
can't be sure what the execution order is, and it's only useful if it's used
consistently in all locations (otherwise anything added to the namespace risks
being overwritten by the one location that doesn't use ||). You mentioned that
you're pulling in the _proto.js files; in that case, it might be better to
assert that the namespace is defined here.

[kelvinp, 2014/07/21 17:46:22]

Done.

+    sinon.stub(chrome.i18n, 'getMessage');
+  },
+  teardown: function() {
+    chrome.i18n.getMessage.restore();
+  }
+});
+
+test('getTranslationOrError(tag) should return tag on error', function() {
+  var translation = l10n.getTranslationOrError('not_existing_tag');

[Jamie, 2014/07/18 02:04:51]

s/no_existing/non_existent/?

[kelvinp, 2014/07/21 17:46:22]

Done.

+  equal(translation, 'not_existing_tag');
+});
+
+test('localizeElementFromTag() should replace innerText by default',
+  function() {
+    var element = document.createElement('div');
+    chrome.i18n.getMessage.withArgs('tag').returns('<b>Hello World</b>');
+
+    l10n.localizeElementFromTag(element, 'tag');
+
+    equal(element.innerText, '<b>Hello World</b>');
+    equal(element.innerHTML, '&lt;b&gt;Hello World&lt;/b&gt;');

[Jamie, 2014/07/18 02:04:51]

Why test both of these? Doesn't the first test imply the second? I think testing
innerHTML in all these test cases will suffice.

[kelvinp, 2014/07/21 17:46:22]

Done.

+});
+
+test('localizeElementFromTag() should replace innerHTML if flag is set',
+  function() {
+    var element = document.createElement('div');
+    chrome.i18n.getMessage.withArgs('tag').returns('<b>Hello World</b>');
+
+    l10n.localizeElementFromTag(element, 'tag', null, true);
+
+    equal(element.innerText, 'Hello World');
+    equal(element.innerHTML, '<b>Hello World</b>');
+});
+
+test(
+  'localizeElement() should replace innerText using the "i18n-content" ' +
+  'attribute as the tag',
+  function() {
+    var element = document.createElement('div');
+    element.setAttribute('i18n-content', 'tag');
+    chrome.i18n.getMessage.withArgs('tag').returns('<b>Hello World</b>');
+
+    l10n.localizeElement(element);
+
+    equal(element.innerText, '<b>Hello World</b>');
+    equal(element.innerHTML, '&lt;b&gt;Hello World&lt;/b&gt;');
+});
+
+test(
+  'localize() should replace element title using the "i18n-title" ' +
+  'attribute as the tag',
+  function() {
+    var fixture = document.getElementById('qunit-fixture');
+    fixture.innerHTML = '<div class="target" i18n-title="tag"></div>';
+    chrome.i18n.getMessage.withArgs('tag').returns('localized title');
+
+    l10n.localize();
+
+    var target = document.querySelector('.target');
+    equal(target.title, 'localized title');
+});
+
+test('localize() should support string substitutions', function() {
+    var fixture = document.getElementById('qunit-fixture');

[Jamie, 2014/07/18 02:04:51]

Nit: indentation.

[kelvinp, 2014/07/21 17:46:22]

Done.

+    fixture.innerHTML = '<div class="target" i18n-content="tag"' +
+                        ' i18n-value-1="param1" i18n-value-2="param2"></div>';

[Jamie, 2014/07/18 02:04:50]

For inline HTML that's too long for one line, I think it's more readable to
format it as you would in an HTML file. See
https://codereview.chromium.org/401623004/diff/20001/remoting/webapp/unittest...,
for example.

[kelvinp, 2014/07/21 17:46:22]

Done.

+
+    chrome.i18n.getMessage.withArgs('tag', ['param1', 'param2'])
+        .returns('localized');
+
+    l10n.localize();
+
+    var target = document.querySelector('.target');
+    equal(target.innerText, 'localized');
+});
+
+test('localize() should support tag substitutions', function() {
+    var fixture = document.getElementById('qunit-fixture');
+    fixture.innerHTML =
+        '<div class="target" i18n-content="tag"' +
+        ' i18n-value-name-1="tag1" i18n-value-name-2="tag2"></div>';
+
+    var getMessage = chrome.i18n.getMessage;
+    getMessage.withArgs('tag1').returns('param1');
+    getMessage.withArgs('tag2').returns('param2');
+    getMessage.withArgs('tag', ['param1', 'param2']).returns('localized');
+
+    l10n.localize();
+
+    var target = document.querySelector('.target');
+    equal(target.innerText, 'localized');
+});
+
+})();