Cloud Print: Add human readable names for standard media sizes.

chrome/browser/resources/print_preview/data/destination_store.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('print_preview', function() {
   'use strict';
 
   /**
    * A data store that stores destinations and dispatches events when the data
    * store changes.
@@ skipping 329 matching lines @@
   DestinationStore.localizeCapabilities_ = function(capabilities) {
     var mediaSize = capabilities.printer.media_size;
     if (mediaSize) {
       var mediaDisplayNames = {
         'ISO_A0': 'A0',
         'ISO_A1': 'A1',
         'ISO_A2': 'A2',
         'ISO_A3': 'A3',
         'ISO_A4': 'A4',
         'ISO_A5': 'A5',
+        'ISO_A6': 'A6',
+        'JIS_B5': 'B5 (JIS)',
+        'NA_EXECUTIVE': 'Executive',
         'NA_LEGAL': 'Legal',
         'NA_LETTER': 'Letter',
-        'NA_LEDGER': 'Tabloid'
+        'NA_LEDGER': 'Tabloid',
+        'OM_FOLIO': 'Folio'
       };
       for (var i = 0, media; media = mediaSize.option[i]; i++) {
         // No need to patch capabilities with localized names provided.
         if (!media.custom_display_name_localized) {
           media.custom_display_name =
               media.custom_display_name ||
               mediaDisplayNames[media.name] ||
               media.name;
         }
       }
     }
     return capabilities;
   };
 
+  /**
+   * Compare two media sizes by their names.
+   * @param {string} a Media to compare.

[dpapad, 2016/10/19 18:30:17]

If |a|, |b| are of type string, they can't have 'custom_display_name_localized'
member var. The type must be something else here.

[Lei Zhang, 2016/10/19 18:59:27]

"!Object" it is.

+   * @param {string} b Media to compare.
+   * @return {number} 1 if a > b, -1 if a < b, or 0 if a == b.
+   * @private
+   */
+  DestinationStore.compareMediaNames_ = function(a, b) {
+    var nameA = a.custom_display_name_localized || a.custom_display_name;
+    var nameB = b.custom_display_name_localized || b.custom_display_name;
+    if (nameA > nameB)

[dpapad, 2016/10/19 18:57:50]

Nit: This can be compressed as follows.

return nameA == nameB ? 0 : (nameA > nameB ? 1 : -1);

[dpapad, 2016/10/19 19:12:31]

^^

[Lei Zhang, 2016/10/19 19:17:26]

Woo, ternary operators.

+      return 1;
+    if (nameA < nameB)
+      return -1;
+    return 0;
+  };
+
+  /**
+   * Sort printer media sizes.
+   * @param {!Object} capabilities Printer capabilities to localize.
+   * @return {!Object} Localized capabilities.
+   * @private
+   */
+  DestinationStore.sortMediaSizes_ = function(capabilities) {
+    var mediaSize = capabilities.printer.media_size;
+    if (mediaSize) {

[dpapad, 2016/10/19 18:30:17]

Nit (optional): Instead of placing the main body of this function in an if()
{...}, consider reversing as follows. IMO is more readable this way (the
tradeoff is repeating the return statement, vs having most of the code indented
by extra 2 spaces.

if (!mediaSize)
  return capabilities;

// remaining logic here.
.....
return capabilities;

[Lei Zhang, 2016/10/19 18:59:27]

Sure. We can do early returns. This was copy + pasted from another function.

+      // For the standard sizes, separate into categories, as seen in the Cloud
+      // Print CDD guide:
+      // - North American
+      // - Chinese
+      // - ISO
+      // - Japanese
+      // - Other metric
+      // Otherwise, assume they are custom sizes.
+      var categoryStandardNA = [];
+      var categoryStandardCN = [];
+      var categoryStandardISO = [];
+      var categoryStandardJP = [];
+      var categoryStandardMisc = [];
+      var categoryCustom = [];
+      for (var i = 0, media; media = mediaSize.option[i]; i++) {
+        var name = media.name;
+        var category;
+        if (name.startsWith('NA_')) {
+          category = categoryStandardNA;
+        } else if (name.startsWith('PRC_') || name.startsWith('ROC_') ||
+                   name == 'OM_DAI_PA_KAI' || name == 'OM_JUURO_KU_KAI' ||
+                   name == 'OM_PA_KAI') {
+          category = categoryStandardCN;
+        } else if (name.startsWith('ISO_')) {
+          category = categoryStandardISO;
+        } else if (name.startsWith('JIS_') || name.startsWith('JPN_')) {
+          category = categoryStandardJP;
+        } else if (name.startsWith('OM_')) {
+          category = categoryStandardMisc;
+        } else {
+          assert(name == 'CUSTOM', 'Unknown media size. Assuming custom');
+          category = categoryCustom;
+        }
+        category.push(media);
+      }
+
+      // For each category, sort by name.
+      categoryStandardNA.sort(DestinationStore.compareMediaNames_);
+      categoryStandardCN.sort(DestinationStore.compareMediaNames_);
+      categoryStandardISO.sort(DestinationStore.compareMediaNames_);
+      categoryStandardJP.sort(DestinationStore.compareMediaNames_);
+      categoryStandardMisc.sort(DestinationStore.compareMediaNames_);
+      categoryCustom.sort(DestinationStore.compareMediaNames_);
+
+      // Then put it all back together.
+      var combined = [];
+      combined = combined.concat(categoryStandardNA);

[dpapad, 2016/10/19 18:30:17]

Array#concat() is creating a new array every time. You could save some copies by
using the 1st array as the starting point and concat all other arrays onto that
one, as follows,

var combined = categoryStandardNA;
combined.push(...categoryStandardCN);
combined.push(...categoryStandardISO);
combined.push(...categoryStandardJP);
combined.push(...categoryStandardMisc);
combined.push(...categoryCustom);
mediaSize.option = combined;

"..." is a new ES6 spread operator, which basically distributes an array as
positional arguments.

[Lei Zhang, 2016/10/19 18:59:27]

Done.

+      combined = combined.concat(categoryStandardCN);
+      combined = combined.concat(categoryStandardISO);
+      combined = combined.concat(categoryStandardJP);
+      combined = combined.concat(categoryStandardMisc);
+      combined = combined.concat(categoryCustom);
+      mediaSize.option = combined;
+    }
+    return capabilities;
+  };
+
   DestinationStore.prototype = {
     __proto__: cr.EventTarget.prototype,
 
     /**
      * @param {string=} opt_account Account to filter destinations by. When
      *     omitted, all destinations are returned.
      * @return {!Array<!print_preview.Destination>} List of destinations
      *     accessible by the {@code account}.
      */
     destinations: function(opt_account) {
@@ skipping 683 matching lines @@
       }
     },
 
     /**
      * Dispatches DESTINATIONS_INSERTED event. In auto select mode, tries to
      * update selected destination to match
      * {@code autoSelectMatchingDestination_}.
      * @param {print_preview.Destination=} opt_destination The only destination
      *     that was changed or skipped if possibly more than one destination was
      *     changed. Used as a hint to limit destination search scope against
-     *     {@code autoSelectMatchingDestination_).
+     *     {@code autoSelectMatchingDestination_}.
      */
     destinationsInserted_: function(opt_destination) {
       cr.dispatchSimpleEvent(
           this, DestinationStore.EventType.DESTINATIONS_INSERTED);
       if (this.autoSelectMatchingDestination_) {
         var destinationsToSearch =
             opt_destination && [opt_destination] || this.destinations_;
         destinationsToSearch.some(function(destination) {
           if (this.autoSelectMatchingDestination_.match(destination)) {
             this.selectDestination(destination);
             return true;
           }
         }, this);
       }
     },
 
     /**
      * Updates an existing print destination with capabilities and display name
      * information. If the destination doesn't already exist, it will be added.
      * @param {!print_preview.Destination} destination Destination to update.
-     * @return {!print_preview.Destination} The existing destination that was
-     *     updated or {@code null} if it was the new destination.
      * @private
      */
     updateDestination_: function(destination) {
       assert(destination.constructor !== Array, 'Single printer expected');
+      destination.capabilities_ = DestinationStore.localizeCapabilities_(
+          destination.capabilities_);
+      destination.capabilities_ = DestinationStore.sortMediaSizes_(
+          destination.capabilities_);
       var existingDestination = this.destinationMap_[this.getKey_(destination)];
       if (existingDestination != null) {
         existingDestination.capabilities = destination.capabilities;
       } else {
         this.insertDestination_(destination);
       }
 
       if (existingDestination == this.selectedDestination_ ||
           destination == this.selectedDestination_) {
         this.appState_.persistSelectedDestination(this.selectedDestination_);
         cr.dispatchSimpleEvent(
             this,
             DestinationStore.EventType.SELECTED_DESTINATION_CAPABILITIES_READY);
       }
-
-      return existingDestination;
     },
 
     /**
      * Called when the search for Privet printers is done.
      * @private
      */
     endPrivetPrinterSearch_: function() {
       this.nativeLayer_.stopGetPrivetDestinations();
       this.isPrivetDestinationSearchInProgress_ = false;
       this.hasLoadedAllPrivetDestinations_ = true;
@@ skipping 380 matching lines @@
       return this.getDestinationKey_(
           destination.origin, destination.id, destination.account);
     }
   };
 
   // Export
   return {
     DestinationStore: DestinationStore
   };
 });