internet_detail.js: Handle undefined APN fields.

chrome/browser/resources/options/chromeos/internet_detail.js

Index: chrome/browser/resources/options/chromeos/internet_detail.js
diff --git a/chrome/browser/resources/options/chromeos/internet_detail.js b/chrome/browser/resources/options/chromeos/internet_detail.js
index 05958eb6a3a4f721c24a92a20c708c7a090311b0..088330729b09fba16635619b44ac736621020944 100644
--- a/chrome/browser/resources/options/chromeos/internet_detail.js
+++ b/chrome/browser/resources/options/chromeos/internet_detail.js
@@ -63,6 +63,16 @@ cr.define('options.internet', function() {
     DetailsInternetPage.getInstance().updateControls();
   }
 
+  /**
+   * Simple helper method for converting a field to a string. It is used to
+   * easily assign an empty string from fields that may be unknown or undefined.
+   * @param {object} value that should be converted to a string.
+   * @return {string} the result.
+   */
+  function stringFromValue(value) {
+    return value ? String(value) : '';
+  }
+
   /////////////////////////////////////////////////////////////////////////////
   // DetailsInternetPage class:
 
@@ -160,15 +170,14 @@ cr.define('options.internet', function() {
 
         if (data.providerApnList.value.length > 0) {
           var iApn = 0;
-          data.apn.apn = data.providerApnList.value[iApn].apn;
-          var username = data.providerApnList.value[iApn].username;
-          var password = data.providerApnList.value[iApn].password;
-          data.apn.username = username ? username : '';
-          data.apn.password = password ? password : '';
+          var defaultApn = data.providerApnList.value[iApn];
+          data.apn.apn = stringFromValue(defaultApn.apn);
+          data.apn.username = stringFromValue(defaultApn.username);
+          data.apn.password = stringFromValue(defaultApn.password);
           chrome.send('setApn', [data.servicePath,
-                                 String(data.apn.apn),
-                                 String(data.apn.username),
-                                 String(data.apn.password)]);
+                                 data.apn.apn,
+                                 data.apn.username,
+                                 data.apn.password]);
           apnSelector.selectedIndex = iApn;
           data.selectedApn = iApn;
         } else {
@@ -189,13 +198,18 @@ cr.define('options.internet', function() {
         var data = $('connection-state').data;
         var apnSelector = $('select-apn');
 
-        data.apn.apn = String($('cellular-apn').value);
-        data.apn.username = String($('cellular-apn-username').value);
-        data.apn.password = String($('cellular-apn-password').value);
+        data.apn.apn = stringFromValue($('cellular-apn').value);
+        data.apn.username = stringFromValue($('cellular-apn-username').value);
+        data.apn.password = stringFromValue($('cellular-apn-password').value);
+        data.userApn = {
+          'apn': data.apn.apn,
+          'username': data.apn.username,
+          'password': data.apn.password
+        };

[stevenjb, 2013/09/03 21:44:32]

nit/question: Could this just be data.userApn = data.apn?

[armansito, 2013/09/03 22:07:38]

I think the code just modifies data.apn instead of creating it from scratch
every time the user sets it. So, we either change that to create a new
dictionary for data.apn every time, or we make a deep copy here. I chose to do a
deep copy here as it seemed simpler to me.

We should probably make this code more modular in general as you rewrite the
Settings UI (with more helpers and better documentation). Also, feel free to
assign any bugs for reimplementing cellular-specific pieces of the new Settings
UI to me (such as APN and SIM unlock). Hopefully we'll have the missing
networkingPrivate pieces ready by then, which is next on my plate.

         chrome.send('setApn', [data.servicePath,
-                               String(data.apn.apn),
-                               String(data.apn.username),
-                               String(data.apn.password)]);
+                               data.apn.apn,
+                               data.apn.username,
+                               data.apn.password]);
 
         if (data.userApnIndex != -1) {
           apnSelector.remove(data.userApnIndex);
@@ -226,23 +240,21 @@ cr.define('options.internet', function() {
         if (apnSelector[apnSelector.selectedIndex].value != -1) {
           var apnList = data.providerApnList.value;
           chrome.send('setApn', [data.servicePath,
-              String(apnList[apnSelector.selectedIndex].apn),
-              String(apnList[apnSelector.selectedIndex].username),
-              String(apnList[apnSelector.selectedIndex].password)
-          ]);
+              stringFromValue(apnList[apnSelector.selectedIndex].apn),
+              stringFromValue(apnList[apnSelector.selectedIndex].username),
+              stringFromValue(apnList[apnSelector.selectedIndex].password)]
+          );
           data.selectedApn = apnSelector.selectedIndex;
         } else if (apnSelector.selectedIndex == data.userApnIndex) {
           chrome.send('setApn', [data.servicePath,
-                                 String(data.apn.apn),
-                                 String(data.apn.username),
-                                 String(data.apn.password)]);
+                                 stringFromValue(data.userApn.apn),
+                                 stringFromValue(data.userApn.username),
+                                 stringFromValue(data.userApn.password)]);
           data.selectedApn = apnSelector.selectedIndex;
         } else {
-          $('cellular-apn').value = data.apn.apn;
-          var username = data.apn.username;
-          var password = data.apn.password;
-          $('cellular-apn-username').value = username ? username : '';
-          $('cellular-apn-password').value = password ? password : '';
+          $('cellular-apn').value = stringFromValue(data.apn.apn);
+          $('cellular-apn-username').value = stringFromValue(data.apn.username);
+          $('cellular-apn-password').value = stringFromValue(data.apn.password);
 
           updateHidden('.apn-list-view', true);
           updateHidden('.apn-details-view', false);
@@ -1126,8 +1138,8 @@ cr.define('options.internet', function() {
       })();
       detailsPage.gsm = data.gsm;
       if (data.gsm) {
-        $('iccid').textContent = data.iccid ? data.iccid : '';
-        $('imsi').textContent = data.imsi ? data.imsi : '';
+        $('iccid').textContent = stringFromValue(data.iccid);
+        $('imsi').textContent = stringFromValue(data.imsi);
 
         var apnSelector = $('select-apn');
         // Clear APN lists, keep only last element that "other".