internet_detail.js: Handle undefined APN fields.

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

 // Copyright (c) 2012 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.
 
 cr.define('options.internet', function() {
   var OptionsPage = options.OptionsPage;
   /** @const */ var ArrayDataModel = cr.ui.ArrayDataModel;
   /** @const */ var IPAddressField = options.internet.IPAddressField;
 
   /**
@@ skipping 45 matching lines @@
   }
 
   /**
    * UI pref change handler.
    * @param {Event} e The update event.
    */
   function handlePrefUpdate(e) {
     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:
 
   /**
    * Encapsulated handling of ChromeOS internet details overlay page.
    * @constructor
    */
   function DetailsInternetPage() {
     OptionsPage.call(this,
                      'detailsInternetPage',
@@ skipping 77 matching lines @@
         var data = $('connection-state').data;
         var apnSelector = $('select-apn');
 
         if (data.userApnIndex != -1) {
           apnSelector.remove(data.userApnIndex);
           data.userApnIndex = -1;
         }
 
         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 {
           data.apn.apn = '';
           data.apn.username = '';
           data.apn.password = '';
           apnSelector.selectedIndex = -1;
           data.selectedApn = -1;
         }
         updateHidden('.apn-list-view', false);
         updateHidden('.apn-details-view', true);
       });
 
       $('cellular-apn-set').addEventListener('click', function(event) {
         if ($('cellular-apn').value == '')
           return;
 
         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);
           data.userApnIndex = -1;
         }
 
         var option = document.createElement('option');
         option.textContent = data.apn.apn;
         option.value = -1;
         option.selected = true;
@@ skipping 10 matching lines @@
         updateHidden('.apn-list-view', false);
         updateHidden('.apn-details-view', true);
       });
 
       $('select-apn').addEventListener('change', function(event) {
         var data = $('connection-state').data;
         var apnSelector = $('select-apn');
         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);
         }
       });
 
       $('sim-card-lock-enabled').addEventListener('click', function(event) {
         var newValue = $('sim-card-lock-enabled').checked;
         // Leave value as is because user needs to enter PIN code first.
         // When PIN will be entered and value changed,
@@ skipping 863 matching lines @@
             $(property).parentElement.hidden = true;
         };
         setContentOrHide('esn');
         setContentOrHide('imei');
         setContentOrHide('meid');
         setContentOrHide('min');
         setContentOrHide('prl-version');
       })();
       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".
         while (apnSelector.length != 1)
           apnSelector.remove(0);
         var otherOption = apnSelector[0];
         data.selectedApn = -1;
         data.userApnIndex = -1;
         var apnList = data.providerApnList.value;
         for (var i = 0; i < apnList.length; i++) {
@@ skipping 99 matching lines @@
 
     // Don't show page name in address bar and in history to prevent people
     // navigate here by hand and solve issue with page session restore.
     OptionsPage.showPageByName('detailsInternetPage', false);
   };
 
   return {
     DetailsInternetPage: DetailsInternetPage
   };
 });