JS style nits

chrome/browser/resources/feedback.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.
 
 // Constants.
 var FEEDBACK_LANDING_PAGE =

[Dan Beam, 2012/04/05 22:24:13]

/** @const */

[Tyler Breisacher (Chromium), 2012/04/05 22:49:55]

Here, yes. The other constants are not actually constant -- it looks they're set
to '' here and then init'd to their real value later. So we should maybe
refactor in another CL...

[Dan Beam, 2012/04/06 02:58:29]

Only meant this one.

-    'http://www.google.com/support/chrome/go/feedback_confirmation'
+    'http://www.google.com/support/chrome/go/feedback_confirmation';
 
 var selectedThumbnailDivId = '';
 var selectedThumbnailId = '';
 var selectedImageUrl;
 
 var savedThumbnailIds = [];
 savedThumbnailIds['current-screenshots'] = '';
 savedThumbnailIds['saved-screenshots'] = '';
 
-var categoryTag = "";
+var categoryTag = '';
 
 var localStrings = new LocalStrings();
 
 /**
  * Selects an image thumbnail in the specified div.
  */
 function selectImage(divId, thumbnailId) {
   var thumbnailDivs = $(divId).children;
   selectedThumbnailDivId = divId;
   if (thumbnailDivs.length == 0) {
@@ skipping 46 matching lines @@
 }
 
 /**
  * Disables screenshots completely.
  */
 function disableScreenshots() {
   $('screenshot-row').hidden = true;
   $('screenshot-checkbox').checked = false;
 
   $('current-screenshots').hidden = true;
   if ($('saved-screenshots'))

[Dan Beam, 2012/04/05 22:24:13]

I'll let this double $('id') lookup slide.

     $('saved-screenshots').hidden = true;
 }
 
 /**
  * Send's the report; after the report is sent, we need to be redirected to
  * the landing page, but we shouldn't be able to navigate back, hence
  * we open the landing page in a new tab and sendReport closes this tab.
  */
 function sendReport() {
   if ($('description-text').value.length == 0) {
     alert(localStrings.getString('no-description'));
     return false;
   }
 
   var imagePath = '';
   if ($('screenshot-checkbox').checked && selectedThumbnailId)
     imagePath = $(selectedThumbnailId + '-image').src;
   var pageUrl = $('page-url-text').value;
   if (!$('page-url-checkbox').checked)
     pageUrl = '';
 
   var reportArray = [pageUrl,
                      categoryTag,
                      $('description-text').value,
                      imagePath];
 
   // Add chromeos data if it exists.
   if ($('user-email-text') && $('sys-info-checkbox')) {
-    var userEmail= $('user-email-text').textContent;
+    var userEmail = $('user-email-text').textContent;

[Dan Beam, 2012/04/05 22:24:13]

and this one, >:|

     if (!$('user-email-checkbox').checked)
       userEmail = '';
     reportArray = reportArray.concat([userEmail,
                                       String($('sys-info-checkbox').checked)]);
   }
 
   // open the landing page in a new tab, sendReport will close this one.
   window.open(FEEDBACK_LANDING_PAGE, '_blank');
   chrome.send('sendReport', reportArray);
   return true;
 }
 
 function cancel() {
-  chrome.send('cancel', []);
+  chrome.send('cancel');
   return true;
 }
 
 /**
  * Select the current screenshots div, restoring the image that was
  * selected when we had this div open previously.
  */
 function currentSelected() {
   // TODO(rkc): Change this to use a class instead.
   $('current-screenshots').hidden = false;
   if ($('saved-screenshots'))
     $('saved-screenshots').hidden = true;

[Dan Beam, 2012/04/05 22:24:13]

and this one, >:|

 
   if (selectedThumbnailDivId != 'current-screenshots')
     selectImage('current-screenshots',

[Dan Beam, 2012/04/05 22:24:13]

and this one, >:|

                 savedThumbnailIds['current-screenshots']);
 
   return true;
 }
 
 /**
  * Select the saved screenshots div, restoring the image that was
  * selected when we had this div open previously.
  */
 function savedSelected() {
   $('current-screenshots').hidden = true;
 
   if ($('saved-screenshots').childElementCount == 0) {
     // setupSavedScreenshots will take care of changing visibility
-    chrome.send('refreshSavedScreenshots', []);
+    chrome.send('refreshSavedScreenshots');
   } else {
     $('saved-screenshots').hidden = false;

[Dan Beam, 2012/04/05 22:24:13]

and this one, >:|

     if (selectedThumbnailDivId != 'saved-screenshots')
       selectImage('saved-screenshots', savedThumbnailIds['saved-screenshots']);
   }
 
   return true;
 }
 
 
 /**
  * Change the type of screenshot we're showing to the user from
@@ skipping 24 matching lines @@
 
   currentSelected();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // Document Functions:
 /**
  * Window onload handler, sets up the page.
  */
 function load() {
   if ($('sysinfo-url')) {

[Dan Beam, 2012/04/05 22:24:13]

and this, :|

     $('sysinfo-url').onclick = function(event) {
       chrome.send('openSystemTab');
     };
   }
 
 <if expr="pp_ifdef('chromeos')">
   $('screenshot-link-tosaved').onclick = changeToSaved;
   $('screenshot-link-tocurrent').onclick = changeToCurrent;
 </if>
   $('send-report-button').onclick = sendReport;
@@ skipping 29 matching lines @@
   // If a page url is spcified in the parameters, override the default page url.
   if (parameters['customPageUrl'] != '') {
     $('page-url-text').value = parameters['customPageUrl'];
     // and disable the page image, since it doesn't make sense on a custum url.
     disableScreenshots();
   }
 
   // Pick up the category tag (for most cases this will be an empty string)
   categoryTag = parameters['categoryTag'];
 
-  chrome.send('getDialogDefaults', []);
-  chrome.send('refreshCurrentScreenshot', []);
-};
+  chrome.send('getDialogDefaults');
+  chrome.send('refreshCurrentScreenshot');
+}
 
 function setupCurrentScreenshot(screenshot) {
   addScreenshot('current-screenshots', screenshot);
 }
 
 function setupSavedScreenshots(screenshots) {
   if (screenshots.length == 0) {
     $('saved-screenshots').textContent =
         localStrings.getString('no-saved-screenshots');
 
@@ skipping 10 matching lines @@
 
     // Now that we have our screenshots, try selecting the saved screenshots
     // again.
     savedSelected();
   }
 }
 
 function setupDialogDefaults(defaults) {
   if (defaults.length > 0) {
     if ($('page-url-text').value == '')
       $('page-url-text').value = defaults[0];

[Dan Beam, 2012/04/05 22:24:13]

and this one, >:|

     if (defaults[0] == '')
       $('page-url-checkbox').checked = false;
 
     if (defaults.length > 2) {
       // We're in Chromium OS.
       $('user-email-text').textContent = defaults[2];
       if (defaults[2] == '') {
         // if we didn't get an e-mail address from cros,
         // disable the user email display totally.
         $('user-email-table').hidden = true;
 
         // this also means we are in privacy mode, so no saved screenshots.
         $('screenshot-link-tosaved').hidden = true;
       }
     }
   }
 }
 
 window.addEventListener('DOMContentLoaded', load);