[MD Bookmarks] Refactor focus ring hiding.

chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js

Index: chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js
diff --git a/chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js b/chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js
index ce62e2aea4a64d09d0d85f39a74661842a396632..2fca111cb8e87d2a76d25018128f8b75202ec1f4 100644
--- a/chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js
+++ b/chrome/browser/resources/md_bookmarks/mouse_focus_behavior.js
@@ -4,63 +4,39 @@
 
 cr.define('bookmarks', function() {
   /** @const */
-  var MOUSE_FOCUS_CLASS = 'mouse-focus';
+  var HIDE_FOCUS_RING_ATTRIBUTE = 'hide-focus-ring';
 
   /**
-   * Behavior which adds the 'mouse-focus' class to a target element when it
-   * gains focus from the mouse, allowing the outline to be hidden without
-   * affecting keyboard users.
+   * Behavior which adds the 'hide-focus-ring' attribute to a target element
+   * when it gains focus from the mouse, allowing the outline to be hidden

[tsergeant, 2017/07/12 07:32:00]

Nit: This comment is a little misleading, since we don't actually listen to
focus anymore. How about:

...when the user interacts with it using the mouse, allowing the focus outline
to be hidden without affecting keyboard users.

[calamity, 2017/07/13 08:03:05]

Done.

+   * without affecting keyboard users.
    * @polymerBehavior
    */
   var MouseFocusBehavior = {
     attached: function() {
       this.boundOnMousedown_ = this.onMousedown_.bind(this);
-      this.boundClearMouseFocus_ = this.clearMouseFocus.bind(this);
+      this.boundsOnKeydown = this.onKeydown_.bind(this);

[tsergeant, 2017/07/12 07:32:00]

Nit: 'bound', not 'bounds'

[calamity, 2017/07/13 08:03:06]

Write once, typo everywhere.

 
-      var target = this.getFocusTarget();
-      target.addEventListener('mousedown', this.boundOnMousedown_);
-      target.addEventListener('contextmenu', this.boundOnMousedown_);
-      target.addEventListener('blur', this.boundClearMouseFocus_);
-    },
-
-    detached: function() {
-      var target = this.getFocusTarget();
-      target.removeEventListener('mousedown', this.boundOnMousedown_);
-      target.removeEventListener('contextmenu', this.boundOnMousedown_);
-      target.removeEventListener('blur', this.boundClearMouseFocus_);
-    },
+      this.addEventListener('mousedown', this.boundOnMousedown_);
+      this.addEventListener('contextmenu', this.boundOnMousedown_);
+      this.addEventListener('keydown', this.boundsOnKeydown, true);
 
-    /**
-     * Returns the element that should be watched for focus changes. Clients
-     * can override, but it is assumed that the return value is constant over
-     * the lifetime of the element.
-     * @return {!HTMLElement}
-     */
-    getFocusTarget: function() {
-      return this;
     },
 
-    /** Reset the focus state when focus leaves the target element. */
-    clearMouseFocus: function() {
-      this.getFocusTarget().classList.remove(MOUSE_FOCUS_CLASS);
+    detached: function() {
+      this.removeEventListener('mousedown', this.boundOnMousedown_);
+      this.removeEventListener('contextmenu', this.boundOnMousedown_);

[tsergeant, 2017/07/12 07:32:00]

Is this necessary? right-clicking will still fire a mousedown event.

[calamity, 2017/07/13 08:03:05]

Removed.

+      this.removeEventListener('keydown', this.boundsOnKeydown, true);
     },
 
     /** @private */
     onMousedown_: function() {
-      this.addMouseFocusClass(this.getFocusTarget());
-    },
-
-    /**
-     * @param {HTMLElement} element
-     * @return {boolean}
-     */
-    isMouseFocused: function(element) {
-      return element.classList.contains(MOUSE_FOCUS_CLASS);
+      this.setAttribute(HIDE_FOCUS_RING_ATTRIBUTE, '');
     },
 
-    /** @param {HTMLElement} element */
-    addMouseFocusClass: function(element) {
-      element.classList.add(MOUSE_FOCUS_CLASS);
+    /** @private */
+    onKeydown_: function() {
+      this.removeAttribute(HIDE_FOCUS_RING_ATTRIBUTE);
     },
   };