Bug 931119 - [SMS] Localize labels in thread header, don't hardcode s…

…eparator character

* Use localizable string to position elements
* Don't hardcode separator characters (' | ', ', ')
* Add 2 tests (different separator characters, fallback for empty separators)

apps/sms/js/utils.js

@@ -80,7 +80,7 @@
     // Please remember to revoke the photoURL after utilizing it.
     getContactDetails:
       function ut_getContactDetails(number, contacts, include) {
-
+      var _ = navigator.mozL10n.get;
       var details = {};
 
       include = include || {};
@@ -142,7 +142,8 @@
         details.org = details.org || org;
 
         if (phone.type) {
-          details.carrier = phone.type + ' | ' + details.carrier;
+          details.carrier =
+            phone.type + (_('thread-separator') || ' | ') + details.carrier;
         }
       }
 
@@ -202,6 +203,7 @@
       var hasUniqueTypes = true;
       var name = hasDetails ? details.name : '';
       var found, tel, type, carrier, value, ending;
+      var _ = navigator.mozL10n.get;
 
       for (var i = 0; i < length; i++) {
         tel = tels[i];
@@ -227,15 +229,24 @@
       }
 
       type = (found.type && found.type[0]) || '';
+      // Non localized label is better than a blank string
+      type = type && _(type) || type;
       carrier = (hasUniqueCarriers || hasUniqueTypes) ? found.carrier : '';
       value = carrier || found.value;
-      ending = ' | ' + (carrier || value);
+      ending = (carrier || value);
 
       if (hasDetails && !name && !carrier) {
         ending = '';
       }
 
-      return type + ending;
+      if (type && ending) {
+        return _('thread-header', {
+          numberType: type,
+          numberDetail: ending
+        });
+      } else {
+        return type + ending;
+      }
     },
 
     // Based on "non-dialables" in https://github.com/andreasgal/PhoneNumber.js
@@ -512,11 +523,15 @@
       all the information needed to display data.
     */
     getDisplayObject: function(theTitle, tel) {
+      var _ = navigator.mozL10n.get;
       var number = tel.value;
       var title = theTitle || number;
       var type = tel.type && tel.type.length ? tel.type[0] : '';
-      var carrier = tel.carrier ? (tel.carrier + ', ') : '';
-      var separator = type || carrier ? ' | ' : '';
+      // For both carrierSeparator and separator we want to avoid using an
+      // empty string as separator because of a bad l10n file.
+      var carrierSeparator = _('carrier-separator') || ', ';
+      var carrier = tel.carrier ? (tel.carrier + carrierSeparator) : '';
+      var separator = type || carrier ? (_('thread-separator') || ' | ') : '';
       var data = {
         name: title,
         number: number,

apps/sms/locales/sms.en-US.properties

@@ -99,6 +99,22 @@ search          = Search
 unknown-contact = Unknown
 carrier-unknown = Carrier unknown
 
+# LOCALIZATION NOTE(thread-header): parameters in this string will be replaced
+# at runtime. {{numberType}} is the label associated to the number ('work',
+# 'home', etc.), {{numberDetail}} is either the phone number or the carrier's
+# name. Resulting example for en-US: "mobile | +123456789", "mobile | Verizon".
+thread-header = {{numberType}} | {{numberDetail}}
+
+# LOCALIZATION NOTE(thread-separator): used to separate type/carrier from
+# the number itself. It should be consistent with the characters used in
+# "thread- header", \u20 is a blank space (\u20|\u20 is equivalent to " | ").
+thread-separator = \u20|\u20
+
+# LOCALIZATION NOTE(carrier-separator): used to separate carrier's name from
+# the number itself. \u20 is a blank space. Resulting example for en-US:
+# "Verizon, +123456789".
+carrier-separator = ,\u20
+
 # Modal Dialogs
 replace-draft               = Replace existing Draft
 save-as-draft               = Save as Draft

apps/sms/test/unit/contact_renderer_test.js

@@ -58,6 +58,12 @@ suite('ContactRenderer', function() {
   setup(function() {
     loadBodyHTML('/index.html');
 
+    // Override generic mozL10n.get for this test
+    var l10nStub = this.sinon.stub(navigator.mozL10n, 'get');
+    l10nStub.withArgs('thread-separator').returns(' | ');
+    l10nStub.withArgs('carrier-separator').returns(', ');
+    l10nStub.returnsArg(0);
+
     this.sinon.spy(Template.prototype, 'interpolate');
     ul = document.createElement('ul');
     contact = MockContact();

apps/sms/test/unit/message_manager_test.js

@@ -450,7 +450,11 @@ suite('message_manager.js >', function() {
     });
 
     suite('message drafts', function() {
+      var nativeMozL10n;
+
       setup(function() {
+        nativeMozL10n = navigator.mozL10n;
+        navigator.mozL10n = MockL10n;
         ThreadUI.draft = new Draft({
           threadId: 1234,
           recipients: []
@@ -463,6 +467,8 @@ suite('message_manager.js >', function() {
       });
 
       teardown(function() {
+        navigator.mozL10n = nativeMozL10n;
+        nativeMozL10n = null;
         ThreadUI.draft = null;
       });
 

apps/sms/test/unit/thread_ui_test.js

@@ -1627,6 +1627,12 @@ suite('thread_ui.js >', function() {
         setup(function() {
           this.sinon.spy(ThreadUI, 'searchContact');
           this.sinon.spy(ThreadUI, 'exactContact');
+
+          // Override generic mozL10n.get for this test
+          var l10nStub = this.sinon.stub(navigator.mozL10n, 'get');
+          l10nStub.withArgs('thread-separator').returns(' | ');
+          l10nStub.withArgs('carrier-separator').returns(', ');
+          l10nStub.returnsArg(0);
         });
 
         test('Triggers assimilation & silent search ', function() {

apps/sms/test/unit/utils_test.js

@@ -34,6 +34,26 @@ suite('Utils', function() {
     navigator.mozL10n = nativeMozL10n;
   });
 
+  setup(function() {
+    // Override generic mozL10n.get for this test
+    this.sinon.stub(navigator.mozL10n, 'get',
+      function get(key, params) {
+        if (params) {
+          if (key == 'thread-header') {
+            return params.numberType + ' | ' + params.numberDetail;
+          }
+          return key + JSON.stringify(params);
+        }
+        if (key == 'thread-separator') {
+          return ' | ';
+        }
+        if (key == 'carrier-separator') {
+          return ', ';
+        }
+        return key;
+    });
+  });
+
   suite('Utils.escapeRegex', function() {
 
     test('functional', function() {
@@ -1008,7 +1028,23 @@ suite('getDisplayObject', function() {
   var nativeMozL10n = navigator.mozL10n;
   setup(function() {
     navigator.mozL10n = MockL10n;
-    this.sinon.spy(navigator.mozL10n, 'get');
+    // Override generic mozL10n.get for this test
+    this.sinon.stub(navigator.mozL10n, 'get',
+      function get(key, params) {
+        if (params) {
+          if (key == 'thread-header') {
+            return params.numberType + ' | ' + params.numberDetail;
+          }
+          return key + JSON.stringify(params);
+        }
+        if (key == 'thread-separator') {
+          return ' | ';
+        }
+        if (key == 'carrier-separator') {
+          return ', ';
+        }
+        return key;
+    });
   });
 
   teardown(function() {
@@ -1082,6 +1118,58 @@ suite('getDisplayObject', function() {
   });
 });
 
+suite('getDisplayObject l10n values', function() {
+  var nativeMozL10n;
+
+  setup(function() {
+    nativeMozL10n = navigator.mozL10n;
+    navigator.mozL10n = MockL10n;
+  });
+
+  teardown(function() {
+    navigator.mozL10n = nativeMozL10n;
+    nativeMozL10n = null;
+  });
+
+  test('(l10n) Empty separator (l10n)', function() {
+    this.sinon.stub(navigator.mozL10n, 'get').returns('');
+    var type = 'Mobile';
+    var carrier = 'Carrier';
+    var value = 111111;
+    var data = Utils.getDisplayObject(null, {
+      'value': value,
+      'carrier': carrier,
+      'type': [type]
+    });
+
+    assert.equal(data.name, value);
+    assert.equal(data.separator, ' | ');
+    assert.equal(data.type, type);
+    assert.equal(data.carrier, carrier + ', ');
+    assert.equal(data.number, value);
+  });
+
+  test('(l10n) Different separators (l10n)', function() {
+    var l10nStub = this.sinon.stub(navigator.mozL10n, 'get');
+    l10nStub.withArgs('thread-separator').returns(' # ');
+    l10nStub.withArgs('carrier-separator').returns(': ');
+    var type = 'Mobile';
+    var carrier = 'Carrier';
+    var value = 111111;
+    var data = Utils.getDisplayObject(null, {
+      'value': value,
+      'carrier': carrier,
+      'type': [type]
+    });
+
+    assert.equal(data.name, value);
+    assert.equal(data.separator, ' # ');
+    assert.equal(data.type, type);
+    assert.equal(data.carrier, carrier + ': ');
+    assert.equal(data.number, value);
+  });
+});
+
 suite('getContactDisplayInfo', function() {
   var nativeMozL10n = navigator.mozL10n;