WebUI: FindShortcutBehavior, cycle through search inputs

Bug: 862839
Change-Id: Ie45e2332b468841cecb3283d93d3761a29326bba
Reviewed-on: https://chromium-review.googlesource.com/c/1394876
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Dan Beam <dbeam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620162}

chrome/browser/resources/settings/languages_page/add_languages_dialog.js

@@ -54,13 +54,19 @@ Polymer({
   handleFindShortcut: function(modalContextOpen) {
     // Assumes this is the only open modal.
     const searchInput = this.$.search.getSearchInput();
-    if (searchInput != this.$.search.shadowRoot.activeElement) {
-      searchInput.scrollIntoViewIfNeeded();
+    searchInput.scrollIntoViewIfNeeded();
+    if (!this.searchInputHasFocus()) {
       searchInput.focus();
     }
     return true;
   },
 
+  // Override FindShortcutBehavior methods.
+  searchInputHasFocus: function() {
+    return this.$.search.getSearchInput() ==
+        this.$.search.shadowRoot.activeElement;
+  },
+
   /**
    * @param {!CustomEvent} e
    * @private

chrome/browser/resources/settings/settings_page/settings_subpage.js

@@ -141,11 +141,13 @@ Polymer({
     if (modalContextOpen) {
       return false;
     }
-    const subpageSearch = this.$$('cr-search-field');
-    const searchInput = subpageSearch.getSearchInput();
-    if (searchInput != subpageSearch.shadowRoot.activeElement) {
-      searchInput.focus();
-    }
+    this.$$('cr-search-field').getSearchInput().focus();
     return true;
   },
+
+  // Override FindShortcutBehavior methods.
+  searchInputHasFocus: function() {
+    const field = this.$$('cr-search-field');
+    return field.getSearchInput() == field.shadowRoot.activeElement;
+  },
 });

chrome/browser/resources/settings/settings_ui/settings_ui.js

@@ -221,6 +221,11 @@ Polymer({
     return true;
   },
 
+  // Override FindShortcutBehavior methods.
+  searchInputHasFocus: function() {
+    return this.$$('cr-toolbar').getSearchField().isSearchFocused();
+  },
+
   /**
    * @param {!CustomEvent} e
    * @private

chrome/test/data/webui/find_shortcut_behavior_test.js

@@ -11,30 +11,9 @@ suite('find-shortcut', () => {
   /**
    * @type {PromiseResolver<!{modalContextOpen: boolean, self: HTMLElement}>}
    */
-  let waits;
-  /** @type {number} */
-  let nextWait;
-
-  /**
-   * Checks that the handleFindShortcut method is being called for all the
-   * |expectedSelves| element references in the order that they are passed in
-   * when a single find shortcut is invoked.
-   * @param {!Array<!HTMLElement>} expectedSelves
-   * @param {?boolean} expectedModalContextOpen
-   * @return {!Promise}
-   */
-  const checkMultiple = async (expectedSelves, expectedModalContextOpen) => {
-    waits = expectedSelves.map(() => new PromiseResolver());
-    nextWait = 0;
-    MockInteractions.pressAndReleaseKeyOn(
-        window, 70, cr.isMac ? 'meta' : 'ctrl', 'f');
-    await Promise.all(waits.map(wait => wait.promise)).then(argss => {
-      argss.forEach((args, index) => {
-        assertEquals(expectedSelves[index], args.self);
-        assertEquals(!!expectedModalContextOpen, args.modalContextOpen);
-      });
-    });
-  };
+  let wait;
+  /** @type {boolean} */
+  let resolved;
 
   /**
    * Checks that the handleFindShortcut method is being called for the
@@ -43,8 +22,15 @@ suite('find-shortcut', () => {
    * @param {?boolean} expectedModalContextOpen
    * @return {!Promise}
    */
-  const check = (expectedSelf, expectedModalContextOpen) =>
-      checkMultiple([expectedSelf], expectedModalContextOpen);
+  const check = async (expectedSelf, expectedModalContextOpen) => {
+    wait = new PromiseResolver();
+    resolved = false;
+    MockInteractions.pressAndReleaseKeyOn(
+        window, 70, cr.isMac ? 'meta' : 'ctrl', 'f');
+    const args = await wait.promise;
+    assertEquals(expectedSelf, args.self);
+    assertEquals(!!expectedModalContextOpen, args.modalContextOpen);
+  };
 
   /**
    * Registers for a keydown event to check whether the bubbled up event has
@@ -69,13 +55,17 @@ suite('find-shortcut', () => {
       behaviors: [FindShortcutBehavior],
 
       findShortcutListenOnAttach: false,
-      handledResponse: true,
+      hasFocus: false,
 
       handleFindShortcut(modalContextOpen) {
-        assert(nextWait < waits.length);
-        waits[nextWait++].resolve({modalContextOpen, self: this});
-        return this.handledResponse;
+        assert(!resolved);
+        wait.resolve({modalContextOpen, self: this});
+        return true;
       },
+
+      searchInputHasFocus() {
+        return this.hasFocus;
+      }
     });
 
     document.body.innerHTML = `
@@ -89,12 +79,17 @@ suite('find-shortcut', () => {
       behaviors: [FindShortcutBehavior],
 
       handledResponse: true,
+      hasFocus: false,
 
       handleFindShortcut(modalContextOpen) {
-        assert(nextWait < waits.length);
-        waits[nextWait++].resolve({modalContextOpen, self: this});
+        assert(!resolved);
+        wait.resolve({modalContextOpen, self: this});
         return this.handledResponse;
       },
+
+      searchInputHasFocus() {
+        return this.hasFocus;
+      },
     });
     PolymerTest.clearBody();
   });
@@ -212,4 +207,30 @@ suite('find-shortcut', () => {
     assertEquals(2, FindShortcutManager.listeners.length);
     await check(testElements[1]);
   });
+
+  test('not handle by listener bubbles up', async () => {
+    const bubbledUp = listenOnceAndCheckDefaultPrevented(false);
+    document.body.innerHTML = `<find-shortcut-element></find-shortcut-element>`;
+    const testElement = document.body.querySelector('find-shortcut-element');
+    testElement.handledResponse = false;
+    await check(testElement);
+    await bubbledUp;
+  });
+
+  test('when element has focus, shortcut is handled by next', async () => {
+    document.body.innerHTML = `
+        <find-shortcut-element></find-shortcut-element>
+        <find-shortcut-element></find-shortcut-element>
+        <find-shortcut-element></find-shortcut-element>`;
+    const testElements =
+        Array.from(document.body.querySelectorAll('find-shortcut-element'));
+    testElements[0].hasFocus = true;
+    await check(testElements[2]);
+    testElements[0].hasFocus = false;
+    testElements[1].hasFocus = true;
+    await check(testElements[0]);
+    testElements[1].hasFocus = false;
+    testElements[2].hasFocus = true;
+    await check(testElements[1]);
+  });
 });

ui/webui/resources/js/find_shortcut_behavior.js

@@ -8,10 +8,11 @@
  * top of the stack will be notified that a find shortcut has been invoked.
  */
 
+
 const FindShortcutManager = (() => {
   /**
    * Stack of listeners. Only the top listener will handle the shortcut.
-   * @type {!Array<!HTMLElement>}
+   * @type {!Array}
    */
   const listeners = [];
 
@@ -27,16 +28,21 @@ const FindShortcutManager = (() => {
       new cr.ui.KeyboardShortcutList(cr.isMac ? 'meta|f' : 'ctrl|f');
 
   window.addEventListener('keydown', e => {
-    if (e.defaultPrevented || listeners.length == 0) {
+    if (e.defaultPrevented || listeners.length == 0 ||
+        !shortcut.matchesEvent(e)) {
       return;
     }
 
-    if (shortcut.matchesEvent(e)) {
-      const listener = /** @type {!{handleFindShortcut: function(boolean)}} */ (
-          listeners[listeners.length - 1]);
-      if (listener.handleFindShortcut(modalContextOpen)) {
-        e.preventDefault();
-      }
+    const focusIndex =
+        listeners.findIndex(listener => listener.searchInputHasFocus());
+    // If no listener has focus or the first (outer-most) listener has focus,
+    // try the last (inner-most) listener.
+    // If a listener has a search input with focus, the next listener that
+    // should be called is the right before it in |listeners| such that the
+    // goes from inner-most to outer-most.
+    const index = focusIndex <= 0 ? listeners.length - 1 : focusIndex - 1;
+    if (listeners[index].handleFindShortcut(modalContextOpen)) {
+      e.preventDefault();
     }
   });
 
@@ -80,16 +86,6 @@ const FindShortcutBehavior = {
     }
   },
 
-  /**
-   * If handled, return true.
-   * @param {boolean} modalContextOpen
-   * @return {boolean}
-   * @protected
-   */
-  handleFindShortcut(modalContextOpen) {
-    assertNotReached();
-  },
-
   becomeActiveFindShortcutListener() {
     const listeners = FindShortcutManager.listeners;
     assert(!listeners.includes(this), 'Already listening for find shortcuts.');
@@ -102,4 +98,22 @@ const FindShortcutBehavior = {
     assert(listeners.includes(this), 'Find shortcut listener not found.');
     listeners.splice(index, 1);
   },
+
+  /**
+   * If handled, return true.
+   * @param {boolean} modalContextOpen
+   * @return {boolean}
+   * @protected
+   */
+  handleFindShortcut(modalContextOpen) {
+    assertNotReached();
+  },
+
+  /**
+   * @return {boolean}
+   * @protected
+   */
+  searchInputHasFocus() {
+    assertNotReached();
+  },
 };