[print preview] Improve appearance of select control separators.

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 518 matching lines @@
 }
 
 /**
  * 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);
+  addDestinationListOption('', '', true, true, false);

[kmadhusu, 2011/10/07 22:11:26]

dpapad: I am not sure why we have a placeholder here. If this is not a separator
we need to add aria label to this after this change. 

[dpapad, 2011/10/10 21:54:27]

Briefly spoke with thestig about why we have this placeholder. It is needed
because otherwise when we try to get the initial print preview we will fail some
checks. So I suggest to leave it as is, and I dont think there is a need to add
aria label since the placeholder is removed and the list is populated before the
user has any chance (normally) to interact with the drop down list.

   if (printer_name) {
     defaultOrLastUsedPrinterName = printer_name;
     if (cloudPrintData) {
       cloudprint.setDefaultPrinter(printer_name,
                                    cloudPrintData,
                                    addDestinationListOptionAtPosition,
                                    doUpdateCloudPrinterCapabilities);
     } else {
       $('printer-list')[0].value = defaultOrLastUsedPrinterName;
       updateControlsWithSelectedPrinterCapabilities();
@@ skipping 60 matching lines @@
  * @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.
  * @return {Object} The created option.
  */
 function createDestinationListOption(optionText, optionValue, isDefault,
     isDisabled, isSeparator) {
+  if (isSeparator)
+    return document.createElement('hr');

[kmadhusu, 2011/10/07 22:11:26]

Can you move lines 620 and 621 to a separate function because it is no longer
required in this function.

You need to call that function from lines 572, 580, 587.

[csilv, 2011/10/07 22:40:34]

Done.

+
   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");
+  option.disabled = isDisabled;
   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 {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.
@@ skipping 390 matching lines @@
 <include src="print_preview_animations.js"/>
 <include src="print_preview_cloud.js"/>
 <include src="print_preview_utils.js"/>
 <include src="print_header.js"/>
 <include src="page_settings.js"/>
 <include src="copies_settings.js"/>
 <include src="header_footer_settings.js"/>
 <include src="layout_settings.js"/>
 <include src="color_settings.js"/>
 <include src="margin_settings.js"/>