downloads: put [column-type] in HTML template instead of JS.

R=hcarmona@chromium.org
BUG=none

Review URL: https://codereview.chromium.org/1000163002

Cr-Commit-Position: refs/heads/master@{#320762}

chrome/browser/resources/downloads/downloads.html

@@ -57,22 +57,26 @@
         <canvas class="progress"></canvas>
         <img class="icon" alt="">
         <div class="title-area">
-          <a is="action-link" class="name"></a>
+          <a is="action-link" class="name" column-type="name"></a>
           <span class="name"></span>
           <span class="status"></span>
         </div>
         <div class="url-container">
-          <a class="src-url" target="_blank"></a>
+          <a class="src-url" target="_blank" column-type="url"></a>
         </div>
         <div class="controls">
-          <a is="action-link" class="show"
+          <a is="action-link" class="show" column-type="show"
               i18n-content="control_showinfolder"></a>
-          <a class="retry" i18n-content="control_retry" download></a>
-          <a is="action-link" class="pause" i18n-content="control_pause"></a>
-          <a is="action-link" class="resume" i18n-content="control_resume"></a>
-          <a is="action-link" class="remove"
+          <a class="retry" column-type="retry" i18n-content="control_retry"
+              download></a>
+          <a is="action-link" class="pause" column-type="pause"
+              i18n-content="control_pause"></a>
+          <a is="action-link" class="resume" column-type="resume"
+              i18n-content="control_resume"></a>
+          <a is="action-link" class="remove" column-type="remove"
               i18n-content="control_removefromlist"></a>
-          <a is="action-link" class="cancel" i18n-content="control_cancel"></a>
+          <a is="action-link" class="cancel" column-type="cancel"
+              i18n-content="control_cancel"></a>
           <span class="controlled-by"
               i18n-values=".innerHTML:control_by_extension"></span>
         </div>
@@ -81,13 +85,15 @@
         <img class="icon" alt="">
         <div class="description"></div>
         <div class="controls">
-          <a is="action-link" class="restore"
+          <a is="action-link" class="restore" column-type="save"
               i18n-content="danger_restore"></a>
-          <a is="action-link" class="remove"
+          <a is="action-link" class="remove" column-type="discard"
               i18n-content="control_removefromlist"></a>
         </div>
-        <button class="save" i18n-content="danger_save"></button>
-        <button class="discard" i18n-content="danger_discard"></button>
+        <button class="save" column-type="save"
+            i18n-content="danger_save"></button>
+        <button class="discard" column-type="discard"
+            i18n-content="danger_discard"></button>
       </div>
     </div>
   </div>

chrome/browser/resources/downloads/focus_row.js

@@ -20,21 +20,7 @@ cr.define('downloads', function() {
   FocusRow.decorate = function(focusRow, itemView, boundary) {
     focusRow.__proto__ = FocusRow.prototype;
     focusRow.decorate(boundary);
-
-    // Add all clickable elements as a row into the grid.
-    focusRow.addElementIfFocusable_(itemView.fileLink, 'name');
-    focusRow.addElementIfFocusable_(itemView.srcUrl, 'url');
-    focusRow.addElementIfFocusable_(itemView.show, 'show');
-    focusRow.addElementIfFocusable_(itemView.retry, 'retry');
-    focusRow.addElementIfFocusable_(itemView.pause, 'pause');
-    focusRow.addElementIfFocusable_(itemView.resume, 'resume');
-    focusRow.addElementIfFocusable_(itemView.safeRemove, 'remove');
-    focusRow.addElementIfFocusable_(itemView.cancel, 'cancel');
-    focusRow.addElementIfFocusable_(itemView.restore, 'save');
-    focusRow.addElementIfFocusable_(itemView.save, 'save');
-    focusRow.addElementIfFocusable_(itemView.dangerRemove, 'discard');
-    focusRow.addElementIfFocusable_(itemView.discard, 'discard');
-    focusRow.addElementIfFocusable_(itemView.controlledBy, 'extension');
+    focusRow.addFocusableElements_();
   };
 
   FocusRow.prototype = {
@@ -50,6 +36,7 @@ cr.define('downloads', function() {
       var equivalent = this.querySelector('[column-type=' + columnType + ']');
 
       if (this.focusableElements.indexOf(equivalent) < 0) {
+        equivalent = null;
         var equivalentTypes =
             ['show', 'retry', 'pause', 'resume', 'remove', 'cancel'];
         if (equivalentTypes.indexOf(columnType) != -1) {
@@ -64,15 +51,13 @@ cr.define('downloads', function() {
       return assert(equivalent || this.focusableElements[0]);
     },
 
-    /**
-     * @param {Element} element The element that should be added.
-     * @param {string} type The column type to use for the element.
-     * @private
-     */
-    addElementIfFocusable_: function(element, type) {
-      if (this.shouldFocus_(element)) {
-        this.addFocusableElement(element);
-        element.setAttribute('column-type', type);
+    /** @private */
+    addFocusableElements_: function() {
+      var possiblyFocusableElements = this.querySelectorAll('[column-type]');
+      for (var i = 0; i < possiblyFocusableElements.length; ++i) {
+        var possiblyFocusableElement = possiblyFocusableElements[i];
+        if (this.shouldFocus_(possiblyFocusableElement))
+          this.addFocusableElement(possiblyFocusableElement);
       }
     },
 

chrome/browser/resources/downloads/item_view.js

@@ -23,39 +23,39 @@ cr.define('downloads', function() {
     this.safeImg_ = /** @type !HTMLImageElement */(
         this.queryRequired_('.safe img'));
     this.fileName_ = this.queryRequired_('span.name');
-    this.fileLink = this.queryRequired_('[is="action-link"].name');
+    this.fileLink_ = this.queryRequired_('[is="action-link"].name');
     this.status_ = this.queryRequired_('.status');
-    this.srcUrl = this.queryRequired_('.src-url');
-    this.show = this.queryRequired_('.show');
-    this.retry = this.queryRequired_('.retry');
-    this.pause = this.queryRequired_('.pause');
-    this.resume = this.queryRequired_('.resume');
-    this.safeRemove = this.queryRequired_('.safe .remove');
-    this.cancel = this.queryRequired_('.cancel');
-    this.controlledBy = this.queryRequired_('.controlled-by');
+    this.srcUrl_ = this.queryRequired_('.src-url');
+    this.show_ = this.queryRequired_('.show');
+    this.retry_ = this.queryRequired_('.retry');
+    this.pause_ = this.queryRequired_('.pause');
+    this.resume_ = this.queryRequired_('.resume');
+    this.safeRemove_ = this.queryRequired_('.safe .remove');
+    this.cancel_ = this.queryRequired_('.cancel');
+    this.controlledBy_ = this.queryRequired_('.controlled-by');
 
     this.dangerous_ = this.queryRequired_('.dangerous');
     this.dangerImg_ = /** @type {!HTMLImageElement} */(
         this.queryRequired_('.dangerous img'));
     this.description_ = this.queryRequired_('.description');
     this.malwareControls_ = this.queryRequired_('.dangerous .controls');
-    this.restore = this.queryRequired_('.restore');
-    this.dangerRemove = this.queryRequired_('.dangerous .remove');
-    this.save = this.queryRequired_('.save');
-    this.discard = this.queryRequired_('.discard');
+    this.restore_ = this.queryRequired_('.restore');
+    this.dangerRemove_ = this.queryRequired_('.dangerous .remove');
+    this.save_ = this.queryRequired_('.save');
+    this.discard_ = this.queryRequired_('.discard');
 
     // Event handlers (bound once on creation).
     this.safe_.ondragstart = this.onSafeDragstart_.bind(this);
-    this.fileLink.onclick = this.onFileLinkClick_.bind(this);
-    this.show.onclick = this.onShowClick_.bind(this);
-    this.pause.onclick = this.onPauseClick_.bind(this);
-    this.resume.onclick = this.onResumeClick_.bind(this);
-    this.safeRemove.onclick = this.onSafeRemoveClick_.bind(this);
-    this.cancel.onclick = this.onCancelClick_.bind(this);
-    this.restore.onclick = this.onRestoreClick_.bind(this);
-    this.save.onclick = this.onSaveClick_.bind(this);
-    this.dangerRemove.onclick = this.onDangerRemoveClick_.bind(this);
-    this.discard.onclick = this.onDiscardClick_.bind(this);
+    this.fileLink_.onclick = this.onFileLinkClick_.bind(this);
+    this.show_.onclick = this.onShowClick_.bind(this);
+    this.pause_.onclick = this.onPauseClick_.bind(this);
+    this.resume_.onclick = this.onResumeClick_.bind(this);
+    this.safeRemove_.onclick = this.onSafeRemoveClick_.bind(this);
+    this.cancel_.onclick = this.onCancelClick_.bind(this);
+    this.restore_.onclick = this.onRestoreClick_.bind(this);
+    this.save_.onclick = this.onSaveClick_.bind(this);
+    this.dangerRemove_.onclick = this.onDangerRemoveClick_.bind(this);
+    this.discard_.onclick = this.onDiscardClick_.bind(this);
   }
 
   /** Progress meter constants. */
@@ -186,8 +186,8 @@ cr.define('downloads', function() {
             data.danger_type == Item.DangerType.POTENTIALLY_UNWANTED;
 
         this.malwareControls_.hidden = !showMalwareControls;
-        this.discard.hidden = showMalwareControls;
-        this.save.hidden = showMalwareControls;
+        this.discard_.hidden = showMalwareControls;
+        this.save_.hidden = showMalwareControls;
       } else {
         var path = encodeURIComponent(data.file_path);
         ItemView.loadScaledIcon(this.safeImg_, 'chrome://fileicon/' + path);
@@ -198,44 +198,45 @@ cr.define('downloads', function() {
         /** @const */ var completelyOnDisk =
             data.state == Item.States.COMPLETE && !data.file_externally_removed;
 
-        this.fileLink.href = data.url;
-        this.ensureTextIs_(this.fileLink, data.file_name);
-        this.fileLink.hidden = !completelyOnDisk;
+        this.fileLink_.href = data.url;
+        this.ensureTextIs_(this.fileLink_, data.file_name);
+        this.fileLink_.hidden = !completelyOnDisk;
 
         /** @const */ var isInterrupted = data.state == Item.States.INTERRUPTED;
         this.fileName_.classList.toggle('interrupted', isInterrupted);
         this.ensureTextIs_(this.fileName_, data.file_name);
         this.fileName_.hidden = completelyOnDisk;
 
-        this.show.hidden = !completelyOnDisk;
+        this.show_.hidden = !completelyOnDisk;
 
-        this.retry.href = data.url;
-        this.retry.hidden = !data.retry;
+        this.retry_.href = data.url;
+        this.retry_.hidden = !data.retry;
 
-        this.pause.hidden = !isInProgress;
+        this.pause_.hidden = !isInProgress;
 
-        this.resume.hidden = !data.resume;
+        this.resume_.hidden = !data.resume;
 
         /** @const */ var isPaused = data.state == Item.States.PAUSED;
         /** @const */ var showCancel = isPaused || isInProgress;
-        this.cancel.hidden = !showCancel;
+        this.cancel_.hidden = !showCancel;
 
-        this.safeRemove.hidden = showCancel ||
+        this.safeRemove_.hidden = showCancel ||
             !loadTimeData.getBoolean('allow_deleting_history');
 
         /** @const */ var controlledByExtension = data.by_ext_id &&
                                                   data.by_ext_name;
-        this.controlledBy.hidden = !controlledByExtension;
+        this.controlledBy_.hidden = !controlledByExtension;
         if (controlledByExtension) {
-          var link = this.controlledBy.querySelector('a');
+          var link = this.controlledBy_.querySelector('a');
           link.href = 'chrome://extensions#' + data.by_ext_id;
+          link.setAttribute('column-type', 'controlled-by');
           link.textContent = data.by_ext_name;
         }
 
         this.ensureTextIs_(this.since_, data.since_string);
         this.ensureTextIs_(this.date_, data.date_string);
-        this.ensureTextIs_(this.srcUrl, data.url);
-        this.srcUrl.href = data.url;
+        this.ensureTextIs_(this.srcUrl_, data.url);
+        this.srcUrl_.href = data.url;
         this.ensureTextIs_(this.status_, this.getStatusText_(data));
 
         this.foregroundProgress_.hidden = !isInProgress;

chrome/browser/resources/downloads/manager.js

@@ -102,7 +102,14 @@ cr.define('downloads', function() {
 
     /** @param {!downloads.Data} data Info about the item to update. */
     updateItem: function(data) {
-      this.idMap_[data.id].render(data);
+      var activeElement = document.activeElement;
+
+      var item = this.idMap_[data.id];
+      item.render(data);
+      var focusRow = this.decorateItem_(item);
+
+      if (activeElement != document.activeElement)
+        focusRow.getEquivalentElement(activeElement).focus();
     },
 
     /**
@@ -117,17 +124,25 @@ cr.define('downloads', function() {
       this.focusGrid_.destroy();
 
       this.items_.forEach(function(item) {
-        downloads.FocusRow.decorate(item.view.node, item.view, this.node_);
-        var focusRow = assertInstanceof(item.view.node, downloads.FocusRow);
+        var focusRow = this.decorateItem_(item);
         this.focusGrid_.addRow(focusRow);
 
-        // Focus the equivalent element in the focusRow because the active
-        // element may no longer be visible.
-        if (focusRow.contains(activeElement))
+        if (activeElement != document.activeElement &&
+            focusRow.contains(activeElement)) {
           focusRow.getEquivalentElement(activeElement).focus();
+        }
       }, this);
     },
 
+    /**
+     * @param {!downloads.Item} item An item to decorate as a FocusRow.
+     * @return {!downloads.FocusRow} |item| decorated as a FocusRow.
+     */
+    decorateItem_: function(item) {
+      downloads.FocusRow.decorate(item.view.node, item.view, this.node_);
+      return assertInstanceof(item.view.node, downloads.FocusRow);
+    },
+
     /** @return {number} The number of downloads shown on the page. */
     size: function() {
       return this.items_.length;

ui/webui/resources/js/cr/ui/focus_grid.js

@@ -64,7 +64,7 @@ cr.define('cr.ui', function() {
       // Only the clicked row should be active.
       var target = assertInstanceof(e.target, Node);
       this.focusGrid_.rows.forEach(function(row) {
-        row.makeRowActive(row.contains(target));
+        row.makeActive(row.contains(target));
       });
 
       return true;
@@ -149,15 +149,12 @@ cr.define('cr.ui', function() {
 
       if (this.rows.length == 0) {
         // The first row should be active if no other row is focused.
-        row.makeRowActive(true);
+        row.makeActive(true);
       } else if (row.contains(document.activeElement)) {
         // The current row should be made active if it's the activeElement.
-        row.makeRowActive(true);
+        row.makeActive(true);
         // Deactivate the first row.
-        this.rows[0].makeRowActive(false);
-      } else {
-        // All other rows should be inactive.
-        row.makeRowActive(false);
+        this.rows[0].makeActive(false);
       }
 
       // Add the row after its initial focus is set.

ui/webui/resources/js/cr/ui/focus_row.js

@@ -54,6 +54,9 @@ cr.define('cr.ui', function() {
     onMousedown: assertNotReached,
   };
 
+  /** @const {string} */
+  FocusRow.ACTIVE_CLASS = 'focus-row-active';
+
   FocusRow.prototype = {
     __proto__: HTMLDivElement.prototype,
 
@@ -105,6 +108,8 @@ cr.define('cr.ui', function() {
       assert(this.focusableElements.indexOf(element) == -1);
       assert(this.contains(element));
 
+      element.tabIndex = this.isActive() ? 0 : -1;
+
       this.focusableElements.push(element);
       this.eventTracker_.add(element, 'mousedown',
                              this.onMousedown_.bind(this));
@@ -118,12 +123,12 @@ cr.define('cr.ui', function() {
      * @private
     */
     onFocusChange_: function(element) {
-      var isActive = this.contains(element);
-      var wasActive = this.classList.contains('focus-row-active');
+      this.makeActive(this.contains(element));
+    },
 
-      // Only send events if the active state is different for the row.
-      if (isActive != wasActive)
-        this.makeRowActive(isActive);
+    /** @return {boolean} Whether this row is currently active. */
+    isActive: function() {
+      return this.classList.contains(FocusRow.ACTIVE_CLASS);
     },
 
     /**
@@ -131,17 +136,21 @@ cr.define('cr.ui', function() {
      * tabIndex can be set properly.
      * @param {boolean} active True if tab is allowed for this row.
      */
-    makeRowActive: function(active) {
+    makeActive: function(active) {
+      if (active == this.isActive())
+        return;
+
       this.focusableElements.forEach(function(element) {
         element.tabIndex = active ? 0 : -1;
       });
 
-      this.classList.toggle('focus-row-active', active);
+      this.classList.toggle(FocusRow.ACTIVE_CLASS, active);
       this.onActiveStateChanged(active);
     },
 
-    /** Call this to clean up event handling before dereferencing. */
+    /** Dereferences nodes and removes event handlers. */
     destroy: function() {
+      this.focusableElements.length = 0;
       this.eventTracker_.removeAll();
     },