[webui][ntp] Add voice search search and link keyboard shortcuts

Adds support to submit a query by pressing <ENTER> or <SPACE>.

We accomplish this by installing a voice search overlay wide keydown
listener to process this keyboard shortcut. Unfortunately, the same
keyboard shortcut also triggers the close button, which was the first
element focused upon opening the overlay before this CL. To submit a
query instead of closing the dialog by default, this CL adds a hidden
focusable div that captures the focus upon opening the dialog. This CL
also turns the close button into a cr-icon-button, which makes the UI
more consistent and makes the button swallow keydown events (that way we
don't both submit a query and close the overlay with <Enter> and
<SPACE>).

+ Add <SPACE> keyboard shortcut to trigger links.

+ Fix bug where any keyboard shortcut other than <CTRL> + <SHIFT> +
  <.> closes the voice search overlay.

Fixed: 1088482
Change-Id: I0c7c40d88503a3ae0d7343a077f8b086968e470b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2238781Reviewed-by: Esmael Elmoslimany <aee@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777474}

chrome/browser/resources/new_tab_page/BUILD.gn

@@ -122,6 +122,8 @@ js_library("grid") {
 js_library("voice_search_overlay") {
   deps = [
     "//third_party/polymer/v3_0/components-chromium/polymer:polymer_bundled",
+    "//ui/webui/resources/cr_elements/cr_button:cr_button.m",
+    "//ui/webui/resources/cr_elements/cr_icon_button:cr_icon_button.m",
   ]
 }
 

chrome/browser/resources/new_tab_page/app.js

@@ -457,18 +457,19 @@ class AppElement extends PolymerElement {
   }
 
   /**
+   * Handles <CTRL> + <SHIFT> + <.> (also <CMD> + <SHIFT> + <.> on mac) to open
+   * voice search.
    * @param {KeyboardEvent} e
    * @private
    */
   onWindowKeydown_(e) {
-    // Open voice search with <CTRL> + <SHIFT> + <.> (also <CMD> + <SHIFT> + <.>
-    // on mac) keyboard shortcut.
     let ctrlKeyPressed = e.ctrlKey;
     // <if expr="is_macosx">
     ctrlKeyPressed = ctrlKeyPressed || e.metaKey;
     // </if>
-    this.showVoiceSearchOverlay_ =
-        ctrlKeyPressed && e.code === 'Period' && e.shiftKey;
+    if (ctrlKeyPressed && e.code === 'Period' && e.shiftKey) {
+      this.showVoiceSearchOverlay_ = true;
+    }
   }
 
   /**

chrome/browser/resources/new_tab_page/voice_search_overlay.html

-<style>
+<style include="cr-icons">
   :host {
     --receiving-audio-color: var(--google-red-refresh-500);
     --speak-shown-duration: 2s;
@@ -28,27 +28,10 @@
   }
 
   #closeButton {
-    background: none;
-    border: none;
-    color: var(--ntp-secondary-text-color);
-    cursor: pointer;
-    font-family: inherit;
-    font-size: 26px;
-    height: 15px;
-    line-height: 0;
-    outline: none;
-    padding: 0;
+    --cr-icon-button-fill-color: var(--ntp-secondary-text-color);
+    margin: 0;
     position: absolute;
     top: 16px;
-    width: 15px;
-  }
-
-  #closeButton:hover {
-    color: var(--ntp-secondary-text-hover-color);
-  }
-
-  :host-context(.focus-outline-visible) #closeButton:focus {
-    box-shadow: var(--ntp-focus-shadow);
   }
 
   :host-context([dir='ltr']) #closeButton {
@@ -188,8 +171,12 @@
     background-color: white;
   }
 </style>
-<dialog id="dialog" on-close="onOverlayClose_" on-click="onOverlayClick_">
-  <button id="closeButton" tabindex="0" title="$i18n{close}">×</button>
+<dialog id="dialog" on-close="onOverlayClose_" on-click="onOverlayClick_"
+    on-keydown="onOverlayKeydown_">
+  <!-- Purely exists to capture focus upon opening the dialog. -->
+  <div tabindex="-1"></div>
+  <cr-icon-button id="closeButton" class="icon-clear" title="$i18n{close}">
+  </cr-icon-button>
   <div id="content">
     <iron-selector id="texts" selected="[[getText_(state_)]]"
         attr-for-selected="text" fallback-selection="none" aria-live="polite"
@@ -218,14 +205,16 @@
         <iron-pages id="errorLinks" selected="[[getErrorLink_(error_)]]"
             attr-for-selected="link" fallback-selection="none">
           <span link="none"></span>
-          <a link="learn-more" target="_blank" href="[[helpUrl_]]"><!--
+          <a link="learn-more" target="_blank" href="[[helpUrl_]]"
+              on-keydown="onLinkKeydown_"><!--
             -->$i18n{learnMore}
           </a>
-          <a link="details" target="_blank" href="[[helpUrl_]]"><!--
+          <a link="details" target="_blank" href="[[helpUrl_]]"
+              on-keydown="onLinkKeydown_"><!--
             -->$i18n{details}
           </a>
           <a link="try-again" id="retryLink" href="#"
-              on-click="onRetryClick_"><!--
+              on-click="onRetryClick_" on-keydown="onLinkKeydown_"><!--
             -->$i18n{tryAgain}
           </a>
         </iron-pages>

chrome/browser/resources/new_tab_page/voice_search_overlay.js

@@ -5,6 +5,7 @@
 import 'chrome://resources/polymer/v3_0/iron-pages/iron-pages.js';
 import 'chrome://resources/polymer/v3_0/iron-selector/iron-selector.js';
 import 'chrome://resources/cr_elements/cr_button/cr_button.m.js';
+import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.m.js';
 
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
@@ -245,6 +246,34 @@ class VoiceSearchOverlayElement extends PolymerElement {
     this.$.dialog.close();
   }
 
+  /**
+   * Handles <ENTER> or <SPACE> to trigger a query if we have recognized speech.
+   * @param {KeyboardEvent} e
+   * @private
+   */
+  onOverlayKeydown_(e) {
+    if (!['Enter', ' '].includes(e.key) || !this.finalResult_) {
+      return;
+    }
+    this.onFinalResult_();
+  }
+
+  /**
+   * Handles <ENTER> or <SPACE> to simulate click.
+   * @param {KeyboardEvent} e
+   * @private
+   */
+  onLinkKeydown_(e) {
+    if (!['Enter', ' '].includes(e.key)) {
+      return;
+    }
+    // Otherwise, we may trigger overlay-wide keyboard shortcuts.
+    e.stopPropagation();
+    // Otherwise, we open the link twice.
+    e.preventDefault();
+    e.target.click();
+  }
+
   /**
    * @param {!Event} e
    * @private

chrome/test/data/webui/new_tab_page/app_test.js

@@ -153,7 +153,8 @@ suite('NewTabPageAppTest', () => {
     assertTrue(!!app.shadowRoot.querySelector('ntp-voice-search-overlay'));
   });
 
-  test('keyboard shortcut opens voice search overlay', async () => {
+  test('voice search keyboard shortcut', async () => {
+    // Test correct shortcut opens voice search.
     // Act.
     window.dispatchEvent(new KeyboardEvent('keydown', {
       ctrlKey: true,
@@ -164,6 +165,18 @@ suite('NewTabPageAppTest', () => {
 
     // Assert.
     assertTrue(!!app.shadowRoot.querySelector('ntp-voice-search-overlay'));
+
+    // Test other shortcut doesn't close voice search.
+    // Act
+    window.dispatchEvent(new KeyboardEvent('keydown', {
+      ctrlKey: true,
+      shiftKey: true,
+      code: 'Enter',
+    }));
+    await flushTasks();
+
+    // Assert.
+    assertTrue(!!app.shadowRoot.querySelector('ntp-voice-search-overlay'));
   });
 
   if (isMac) {

chrome/test/data/webui/new_tab_page/voice_search_overlay_test.js

@@ -2,10 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import {BrowserProxy} from 'chrome://new-tab-page/new_tab_page.js';
+import {$$, BrowserProxy} from 'chrome://new-tab-page/new_tab_page.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {flushTasks, isVisible} from 'chrome://test/test_util.m.js';
-import {assertStyle, createTestProxy} from './test_support.js';
+import {assertStyle, createTestProxy, keydown} from './test_support.js';
+
+function createResults(n) {
+  return {
+    results: Array.from(Array(n)).map(() => {
+      return {
+        isFinal: false,
+        0: {
+          transcript: 'foo',
+          confidence: 1,
+        },
+      };
+    }),
+    resultIndex: 0,
+  };
+}
 
 class MockSpeechRecognition {
   constructor() {
@@ -99,27 +114,13 @@ suite('NewTabPageVoiceSearchOverlayTest', () => {
   test('on result received shows recognized text', () => {
     // Arrange.
     testProxy.setResultFor('random', 0.5);
+    const result = createResults(2);
+    result.results[1][0].confidence = 0;
+    result.results[0][0].transcript = 'hello';
+    result.results[1][0].transcript = 'world';
 
     // Act.
-    mockSpeechRecognition.onresult({
-      results: [
-        {
-          isFinal: false,
-          0: {
-            transcript: 'hello',
-            confidence: 1,
-          },
-        },
-        {
-          isFinal: false,
-          0: {
-            transcript: 'world',
-            confidence: 0,
-          },
-        },
-      ],
-      resultIndex: 0,
-    });
+    mockSpeechRecognition.onresult(result);
 
     // Assert.
     const [intermediateResult, finalResult] =
@@ -139,20 +140,12 @@ suite('NewTabPageVoiceSearchOverlayTest', () => {
     const googleBaseUrl = 'https://google.com';
     loadTimeData.overrideValues({googleBaseUrl: googleBaseUrl});
     testProxy.setResultFor('random', 0);
+    const result = createResults(1);
+    result.results[0].isFinal = true;
+    result.results[0][0].transcript = 'hello world';
 
     // Act.
-    mockSpeechRecognition.onresult({
-      results: [
-        {
-          isFinal: true,
-          0: {
-            transcript: 'hello world',
-            confidence: 1,
-          },
-        },
-      ],
-      resultIndex: 0,
-    });
+    mockSpeechRecognition.onresult(result);
 
     // Assert.
     const href = await testProxy.whenCalled('navigate');
@@ -192,19 +185,12 @@ suite('NewTabPageVoiceSearchOverlayTest', () => {
   });
 
   test('on end received shows result text if final result', () => {
+    // Arrange.
+    const result = createResults(1);
+    result.results[0].isFinal = true;
+
     // Act.
-    mockSpeechRecognition.onresult({
-      results: [
-        {
-          isFinal: true,
-          0: {
-            transcript: 'hello world',
-            confidence: 1,
-          },
-        },
-      ],
-      resultIndex: 0,
-    });
+    mockSpeechRecognition.onresult(result);
     mockSpeechRecognition.onend();
 
     // Assert.
@@ -223,18 +209,7 @@ suite('NewTabPageVoiceSearchOverlayTest', () => {
     },
     {
       functionName: 'onresult',
-      arguments: [{
-        results: [
-          {
-            isFinal: false,
-            0: {
-              transcript: 'hello',
-              confidence: 1,
-            },
-          },
-        ],
-        resultIndex: 0,
-      }],
+      arguments: [createResults(1)],
     },
     {
       functionName: 'onend',
@@ -323,4 +298,46 @@ suite('NewTabPageVoiceSearchOverlayTest', () => {
       assertTrue(mockSpeechRecognition.startCalled);
     });
   });
+
+  [' ', 'Enter'].forEach(key => {
+    test(`'${key}' submits query if result`, () => {
+      // Arrange.
+      mockSpeechRecognition.onresult(createResults(1));
+      assertEquals(0, testProxy.getCallCount('navigate'));
+
+      // Act.
+      keydown(voiceSearchOverlay.shadowRoot.activeElement, key);
+
+      // Assert.
+      assertEquals(1, testProxy.getCallCount('navigate'));
+      assertTrue(voiceSearchOverlay.$.dialog.open);
+    });
+
+    test(`'${key}' does not submit query if no result`, () => {
+      // Act.
+      keydown(voiceSearchOverlay.shadowRoot.activeElement, key);
+
+      // Assert.
+      assertEquals(0, testProxy.getCallCount('navigate'));
+      assertTrue(voiceSearchOverlay.$.dialog.open);
+    });
+
+    test(`'${key}' triggers link`, () => {
+      // Arrange.
+      mockSpeechRecognition.onerror({error: 'audio-capture'});
+      const link = $$(voiceSearchOverlay, '[link=learn-more]');
+      link.href = '#';
+      link.target = '_self';
+      let clicked = false;
+      link.addEventListener('click', () => clicked = true);
+
+      // Act.
+      keydown(link, key);
+
+      // Assert.
+      assertTrue(clicked);
+      assertEquals(0, testProxy.getCallCount('navigate'));
+      assertFalse(voiceSearchOverlay.$.dialog.open);
+    });
+  });
 });