Printer dropdown cleanup

chrome/browser/resources/print_preview/print_preview.js

 // Copyright (c) 2011 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.
 
 // require: cr/ui/print_preview_cloud.js
 
 var localStrings = new LocalStrings();
 
 // If useCloudPrint is true we attempt to connect to cloud print
 // and populate the list of printers with cloud print printers.
@@ skipping 38 matching lines @@
 var copiesSettings;
 
 // Object holding all the layout related settings.
 var layoutSettings;
 
 // True if the user has click 'Advanced...' in order to open the system print
 // dialog.
 var showingSystemDialog = false;
 
 // The range of options in the printer dropdown controlled by cloud print.
-var firstCloudPrintOptionPos = 0
+var firstCloudPrintOptionPos = 0;
 var lastCloudPrintOptionPos = firstCloudPrintOptionPos;
 
+
+// TODO(abodenha@chromium.org) A lot of cloud print specific logic has
+// made its way into this file.  Refactor to create a cleaner boundary
+// between print preview and GCP code.  Reference bug 88098 when fixing.
+
+// A dictionary of cloud printers that have been added to the printer
+// dropdown.
+var addedCloudPrinters = {};
+
+// The maximum number of cloud printers to allow in the dropdown.
+const maxCloudPrinters = 10;
+
+
 /**
  * Window onload handler, sets up the page and starts print preview by getting
  * the printer list.
  */
 function onLoad() {
   cr.enablePlatformSpecificCSSRules();
 
   $('cancel-button').addEventListener('click', handleCancelButtonClick);
 
   if (!checkCompatiblePluginExists()) {
@@ skipping 114 matching lines @@
   if (cloudprint.isCloudPrint(printerList.options[selectedIndex])) {
     updateWithCloudPrinterCapabilities();
     skip_refresh = true;
   } else if (selectedValue == SIGN_IN ||
              selectedValue == MANAGE_CLOUD_PRINTERS ||
              selectedValue == MANAGE_LOCAL_PRINTERS ||
              selectedValue == ADD_CLOUD_PRINTER) {
     printerList.selectedIndex = lastSelectedPrinterIndex;
     chrome.send(selectedValue);
     skip_refresh = true;
+  } else if (selectedValue == MORE_PRINTERS) {
+    onSystemDialogLinkClicked();
+    skip_refresh = true;
   } else if (selectedValue == PRINT_TO_PDF) {
     updateWithPrinterCapabilities({
         'disableColorOption': true,
         'setColorAsDefault': true,
         'setDuplexAsDefault': false,
         'disableCopiesOption': true});
   } else {
     // This message will call back to 'updateWithPrinterCapabilities'
     // function.
     chrome.send('getPrinterCapabilities', [selectedValue]);
@@ skipping 127 matching lines @@
 }
 
 /**
  * @return {number} The next unused preview request id.
  */
 function generatePreviewRequestID() {
   return ++lastPreviewRequestID;
 }
 
 /**
+ * @return {boolean} True iff a preview has been requested.
+ */
+function hasRequestedPreview() {
+  return lastPreviewRequestID > -1;
+}
+
+/**
  * Returns the name of the selected printer or the empty string if no
  * printer is selected.
  * @return {string} The name of the currently selected printer.
  */
 function getSelectedPrinterName() {
   var printerList = $('printer-list')
   var selectedPrinter = printerList.selectedIndex;
   if (selectedPrinter < 0)
     return '';
   return printerList.options[selectedPrinter].value;
@@ skipping 86 matching lines @@
   // TODO(thestig) re-enable controls here.
 }
 
 /**
  * Set the default printer. If there is one, generate a print preview.
  * @param {string} printer Name of the default printer. Empty if none.
  * @param {string} cloudPrintData Cloud print related data to restore if
  *                 the default printer is a cloud printer.
  */
 function setDefaultPrinter(printer_name, cloudPrintData) {
-  // Add a placeholder value so the printer list looks valid.
-  addDestinationListOption('', '', true, true, true);
   if (printer_name) {
     defaultOrLastUsedPrinterName = printer_name;
     if (cloudPrintData) {
       cloudprint.setDefaultPrinter(printer_name,
                                    cloudPrintData,
                                    addCloudPrinters,
                                    doUpdateCloudPrinterCapabilities);
     } else {
-      $('printer-list')[0].value = defaultOrLastUsedPrinterName;
+      addDestinationListOption('',
+                               defaultOrLastUsedPrinterName,
+                               true,
+                               true,
+                               true);
       updateControlsWithSelectedPrinterCapabilities();
     }
   }
   chrome.send('getPrinters');
 }
 
 /**
  * Fill the printer list drop down.
  * Called from PrintPreviewHandler::SendPrinterList().
  * @param {Array} printers Array of printer info objects.
  */
 function setPrinters(printers) {
-  var printerList = $('printer-list');
-  // If there exists a dummy printer value, then setDefaultPrinter() already
-  // requested a preview, so no need to do it again.
-  var needPreview = (printerList[0].value == '');
   for (var i = 0; i < printers.length; ++i) {
     var isDefault = (printers[i].deviceName == defaultOrLastUsedPrinterName);
     addDestinationListOption(printers[i].printerName, printers[i].deviceName,
                              isDefault, false, false);
   }
 
-  if (!cloudprint.isCloudPrint(printerList[0]))
-    // Remove the dummy printer added in setDefaultPrinter().
-    printerList.remove(0);
-
   if (printers.length != 0)
     addDestinationListOption('', '', false, true, true);
 
   // Adding option for saving PDF to disk.
   addDestinationListOption(localStrings.getString('printToPDF'),
                            PRINT_TO_PDF,
                            defaultOrLastUsedPrinterName == PRINT_TO_PDF,
                            false,
                            false);
   addDestinationListOption('', '', false, true, true);
 
   // Add options to manage printers.
   if (!cr.isChromeOS) {
     addDestinationListOption(localStrings.getString('manageLocalPrinters'),
         MANAGE_LOCAL_PRINTERS, false, false, false);
   }
   if (useCloudPrint) {
+    // Fetch recent printers.
     cloudprint.fetchPrinters(addCloudPrinters, false);
+    // Fetch the full printer list.
+    cloudprint.fetchPrinters(addCloudPrinters, true);
     addDestinationListOption(localStrings.getString('manageCloudPrinters'),
         MANAGE_CLOUD_PRINTERS, false, false, false);
   }
 
-  printerList.disabled = false;
+  $('printer-list').disabled = false;
 
-  if (needPreview)
+  if (!hasRequestedPreview())
     updateControlsWithSelectedPrinterCapabilities();
 }
 
 /**
  * Creates an option that can be added to the printer destination list.
- * @param {String} optionText specifies the option text content.
- * @param {String} optionValue specifies the option value.
+ * @param {string} optionText specifies the option text content.
+ * @param {string} optionValue specifies the option value.
  * @param {boolean} isDefault is true if the option needs to be selected.
  * @param {boolean} isDisabled is true if the option needs to be disabled.
  * @param {boolean} isSeparator is true if the option is a visual separator and
- *                  needs to be disabled.
+ *     needs to be disabled.
  * @return {Object} The created option.
  */
 function createDestinationListOption(optionText, optionValue, isDefault,
     isDisabled, isSeparator) {
   var option = document.createElement('option');
   option.textContent = optionText;
   option.value = optionValue;
   option.selected = isDefault;
   option.disabled = isSeparator || isDisabled;
   // Adding attribute for improved accessibility.
   if (isSeparator)
     option.setAttribute("role", "separator");
   return option;
 }
 
 /**
  * Adds an option to the printer destination list.
- * @param {String} optionText specifies the option text content.
- * @param {String} optionValue specifies the option value.
+ * @param {string} optionText specifies the option text content.
+ * @param {string} optionValue specifies the option value.
  * @param {boolean} isDefault is true if the option needs to be selected.
  * @param {boolean} isDisabled is true if the option needs to be disabled.
  * @return {Object} The created option.
  */
 function addDestinationListOption(optionText, optionValue, isDefault,
     isDisabled, isSeparator) {
   var option = createDestinationListOption(optionText,
                                            optionValue,
                                            isDefault,
                                            isDisabled,
                                            isSeparator);
   $('printer-list').add(option);
   return option;
 }
 
 /**
  * Adds an option to the printer destination list at a specified position.
- * @param {Integer} position The index in the printer-list wher the option
-                             should be created.
- * @param {String} optionText specifies the option text content.
- * @param {String} optionValue specifies the option value.
+ * @param {number} position The index in the printer-list wher the option
+       should be created.
+ * @param {string} optionText specifies the option text content.
+ * @param {string} optionValue specifies the option value.
  * @param {boolean} isDefault is true if the option needs to be selected.
  * @param {boolean} isDisabled is true if the option needs to be disabled.
  * @return {Object} The created option.
  */
 function addDestinationListOptionAtPosition(position,
                                             optionText,
                                             optionValue,
                                             isDefault,
                                             isDisabled,
                                             isSeparator) {
   var option = createDestinationListOption(optionText,
                                            optionValue,
                                            isDefault,
                                            isDisabled,
                                            isSeparator);
   var printerList = $('printer-list');
   var before = printerList[position];
   printerList.add(option, before);
   return option;
 }
 
 /**
- * Removes the list of cloud printers from the printer pull down.
+ * Test if a particular cloud printer has already been added to the
+ * printer dropdown.
+ * @param {string} id A unique value to track this printer.
+ * @return {boolean} Returns true if this id has previously

[dpapad, 2011/07/19 23:01:35]

Nit: s/Returns true/True.

[Albert Bodenhamer, 2011/07/19 23:17:33]

Done.

+ *     been passed to trackCloudPrinterAdded.
  */
-function clearCloudPrinters() {
-  var printerList = $('printer-list');
-  while (firstCloudPrintOptionPos < lastCloudPrintOptionPos) {
-    lastCloudPrintOptionPos--;
-    printerList.remove(lastCloudPrintOptionPos);
+function cloudPrinterAlreadyAdded(id) {
+  return (addedCloudPrinters[id]);
+}
+
+/**
+ * Record that a cloud printer will added to the printer dropdown.
+ * @param {string} id A unique value to track this printer.
+ * @return {boolean} Returns false if adding this printer would exceed

[dpapad, 2011/07/19 23:01:35]

Nit: s/Returns false/False 

[Albert Bodenhamer, 2011/07/19 23:17:33]

Done.

+ *     |maxCloudPrinters|.
+ */
+function trackCloudPrinterAdded(id) {
+  if (Object.keys(addedCloudPrinters).length < maxCloudPrinters) {
+    addedCloudPrinters[id] = true;
+    return true;
+  } else {
+    return false;
   }
 }
 
+
 /**
  * Add cloud printers to the list drop down.
  * Called from the cloudprint object on receipt of printer information from the
  * cloud print server.
  * @param {Array} printers Array of printer info objects.
  * @return {Object} The currently selected printer.
  */
-function addCloudPrinters(printers, showMorePrintersOption) {
-  // TODO(abodenha@chromium.org) Avoid removing printers from the list.
-  // Instead search for duplicates and don't add printers that already exist
-  // in the list.
-  clearCloudPrinters();
-  addDestinationListOptionAtPosition(
-      lastCloudPrintOptionPos++,
-      localStrings.getString('cloudPrinters'),
-      '',
-      false,
-      true,
-      false);
+function addCloudPrinters(printers) {
+  var isFirstPass = false;
+  var showMorePrintersOption = false;
+
+  if (firstCloudPrintOptionPos == lastCloudPrintOptionPos) {
+    isFirstPass = true;
+    var option = addDestinationListOptionAtPosition(
+        lastCloudPrintOptionPos++,
+        localStrings.getString('cloudPrinters'),
+        'Label',
+        false,
+        true,
+        false);
+    cloudprint.setCloudPrint(option, null, null);
+  }
   if (printers != null) {
     if (printers.length == 0) {
       addDestinationListOptionAtPosition(lastCloudPrintOptionPos++,
           localStrings.getString('addCloudPrinter'),
           ADD_CLOUD_PRINTER,
           false,
           false,
           false);
     } else {
-      for (printer in printers) {
-        var option = addDestinationListOptionAtPosition(
-            lastCloudPrintOptionPos++,
-            printers[printer]['name'],
-            printers[printer]['id'],
-            printers[printer]['name'] == defaultOrLastUsedPrinterName,
-            false,
-            false);
-        cloudprint.setCloudPrint(option,
-                                 printers[printer]['name'],
-                                 printers[printer]['id']);
+      for (var i = 0; i < printers.length; i++) {
+        if (!cloudPrinterAlreadyAdded(printers[i]['id'])) {
+          var option = addDestinationListOptionAtPosition(
+              lastCloudPrintOptionPos++,
+              printers[i]['name'],
+              printers[i]['id'],
+              printers[i]['name'] == defaultOrLastUsedPrinterName,
+              false,
+              false);
+          cloudprint.setCloudPrint(option,
+                                   printers[i]['name'],
+                                   printers[i]['id']);
+          if (!trackCloudPrinterAdded(printers[i]['id'])) {
+            showMorePrintersOption = true;
+            break;
+          }
+        }
       }
       if (showMorePrintersOption) {
         addDestinationListOptionAtPosition(lastCloudPrintOptionPos++,
                                            '',
                                            '',
                                            false,
                                            true,
                                            true);
         addDestinationListOptionAtPosition(lastCloudPrintOptionPos++,
             localStrings.getString('morePrinters'),
             MORE_PRINTERS, false, false, false);
       }
     }
   } else {
-    addDestinationListOptionAtPosition(lastCloudPrintOptionPos++,
-                                       localStrings.getString('signIn'),
-                                       SIGN_IN,
+    if (!cloudPrinterAlreadyAdded(SIGN_IN)) {
+      addDestinationListOptionAtPosition(lastCloudPrintOptionPos++,
+                                         localStrings.getString('signIn'),
+                                         SIGN_IN,
+                                         false,
+                                         false,
+                                         false);
+      trackCloudPrinterAdded(SIGN_IN);
+    }
+  }
+  if (isFirstPass) {
+    addDestinationListOptionAtPosition(lastCloudPrintOptionPos,
+                                       '',
+                                       '',
                                        false,
+                                       true,
+                                       true);
+    addDestinationListOptionAtPosition(lastCloudPrintOptionPos + 1,
+                                       localStrings.getString('localPrinters'),
+                                       '',
                                        false,
+                                       true,
                                        false);
   }
-  addDestinationListOptionAtPosition(lastCloudPrintOptionPos++,
-                                     '',
-                                     '',
-                                     false,
-                                     true,
-                                     true);
-  addDestinationListOptionAtPosition(lastCloudPrintOptionPos++,
-                                     localStrings.getString('localPrinters'),
-                                     '',
-                                     false,
-                                     true,
-                                     false);
   var printerList = $('printer-list')
   var selectedPrinter = printerList.selectedIndex;
   if (selectedPrinter < 0)
     return null;
   return printerList.options[selectedPrinter];
 }
 
 /**
  * Sets the color mode for the PDF plugin.
  * Called from PrintPreviewHandler::ProcessColorSetting().
@@ skipping 272 matching lines @@
 }
 
 /**
  * Takes a snapshot of the print settings.
  */
 PrintSettings.prototype.save = function() {
   this.deviceName = getSelectedPrinterName();
   this.isLandscape = layoutSettings.isLandscape();
 }