MD Settings: fix focus-row-behavior allowing invisible focus on rows.

chrome/browser/resources/settings/focus_row_behavior.js

 // Copyright 2017 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.
 
 /**
  * @param {{lastFocused: Object}} listItem
  * @constructor
  * @implements {cr.ui.FocusRow.Delegate}
  */
 function FocusRowDelegate(listItem) {
   /** @private */
   this.listItem_ = listItem;
 }
 
 FocusRowDelegate.prototype = {
   /**
    * @override
    * @param {!cr.ui.FocusRow} row
    * @param {!Event} e
    */
   onFocus: function(row, e) {

[hcarmona, 2017/06/08 18:57:50]

When does this |onFocus| get called?

[scottchen, 2017/06/08 22:55:35]

When a control element (i,e, [focus-row-control]) within the list gets focused.

[hcarmona, 2017/06/08 23:21:31]

Sounds good. Let's add a short comment to both focus functions to remove
ambiguity about when they are called.

[scottchen, 2017/06/09 00:06:00]

Done.

     this.listItem_.lastFocused = e.path[0];
+    this.listItem_.tabIndex = -1;
   },
 
   /**
    * @override
    * @param {!cr.ui.FocusRow} row The row that detected a keydown.
    * @param {!Event} e
    * @return {boolean} Whether the event was handled.
    */
   onKeydown: function(row, e) {
     // Prevent iron-list from changing the focus on enter.
@@ skipping 98 matching lines @@
 
       for (var i = 0; i < controls.length; i++) {
         this.row_.addItem(
             controls[i].getAttribute('focus-type'),
             /** @type {HTMLElement} */ (controls[i]));
       }
     }
   },
 
   /** @private */
   onFocus_: function() {

[hcarmona, 2017/06/08 18:57:50]

Vs this |onFocus_|?

IIUC, they both set tabIndex = -1.

[scottchen, 2017/06/08 22:55:35]

This is when the row gets focused. This function essentially forwards the focus
to the first focusable child and then set the row itself to be unfocusable.

However, there are some scenarios in which the contained control element get
focused programmatically (e.g. when cr-action-menu). 

In such scenario, this function is not invoked, so when you shift-tab from the
child, the focus *would* go to the row because its tabindex is not set to -1
yet, which is undesired.

[hcarmona, 2017/06/08 23:21:31]

Acknowledged.

     if (this.mouseFocused_) {
       this.mouseFocused_ = false;  // Consume and reset flag.
       return;
     }
 
     if (this.lastFocused) {
       this.row_.getEquivalentElement(this.lastFocused).focus();
     } else {
       var firstFocusable = assert(this.row_.getFirstFocusable());
       firstFocusable.focus();
@@ skipping 11 matching lines @@
   /** @private */
   onMouseDown_: function() {
     this.mouseFocused_ = true;  // Set flag to not do any control-focusing.
   },
 
   /** @private */
   onBlur_: function() {
     this.mouseFocused_ = false;  // Reset flag since it's not active anymore.
   }
 };