Bug 1892417 - Make feature callout autofocus configurable and disabled by default. r=omc-reviewers,pdahiya

Differential Revision: https://phabricator.services.mozilla.com/D247391

browser/components/asrouter/docs/feature-callout.md

@@ -217,6 +217,12 @@ interface FeatureCallout {
           // default (this corresponds to a triangle with 24px edges). This
           // also affects the height of the arrow.
           arrow_width?: number;
+          // By default, callouts are not focused when they are shown. The user
+          // must use a mouse or the F6 shortcut to interact with the callout.
+          // This property allows you to force an element inside the callout to
+          // be focused when the callout is shown. Use sparingly, as it can make
+          // callouts much more disruptive for users.
+          autofocus?: AutoFocusOptions;
         }
       ];
       content: {
@@ -410,6 +416,10 @@ interface FeatureCallout {
             dismiss?: boolean;
           };
         }>;
+        // An action to perform when the Escape key is pressed, or when a page
+        // event listener invokes an action containing `dismiss: true`.
+        // Unnecessary if your message has a dismiss_button.
+        dismiss_action?: Action;
       };
     }>;
     // Specify the index of the screen to start on. Generally unused.
@@ -427,6 +437,20 @@ type PopupAttachmentPoint =
   | "topcenter"
   | "bottomcenter";
 
+interface AutoFocusOptions {
+  // A preferred CSS selector, if you want a specific element to be focused. If
+  // omitted, the default prioritization listed below will be used, based on
+  // `use_defaults`.
+  // Default prioritization: primary_button, secondary_button, additional_button
+  //   (excluding pseudo-links), dismiss_button, <input>, any button.
+  selector?: string;
+  // Whether to use the default element prioritization. If `selector` is
+  // provided and the element can't be found, and this is set to false, nothing
+  // will be selected. If `selector` is not provided, this must be true.
+  // Defaults to true.
+  use_defaults?: boolean;
+}
+
 type Label = string | LocalizableThing;
 
 interface LocalizableThing {

browser/components/asrouter/modules/FeatureCallout.sys.mjs

@@ -455,28 +455,16 @@ export class FeatureCallout {
         if (!this._container) {
           return;
         }
-        let focusedElement =
+        this.win.AWSendEventTelemetry?.({
-          this.context === "chrome"
+          event: "DISMISS",
-            ? Services.focus.focusedElement
+          event_context: {
-            : this.doc.activeElement;
+            source: `KEY_${event.key}`,
-        // If the window has a focused element, let it handle the ESC key instead.
+            page: this.location,
-        if (
+          },
-          !focusedElement ||
+          message_id: this.config?.id.toUpperCase(),
-          focusedElement === this.doc.body ||
+        });
-          (focusedElement === this.browser && this.theme.simulateContent) ||
+        this._dismiss();
-          this._container.contains(focusedElement)
+        event.preventDefault();
-        ) {
-          this.win.AWSendEventTelemetry?.({
-            event: "DISMISS",
-            event_context: {
-              source: `KEY_${event.key}`,
-              page: this.location,
-            },
-            message_id: this.config?.id.toUpperCase(),
-          });
-          this._dismiss();
-          event.preventDefault();
-        }
         break;
       }
 
@@ -641,6 +629,19 @@ export class FeatureCallout {
    * @property {String} [bottom]
    */
 
+  /**
+   * @typedef {Object} AutoFocusOptions For the optional autofocus feature.
+   * @property {String} [selector] A preferred CSS selector, if you want a
+   *   specific element to be focused. If omitted, the default prioritization
+   *   listed below will be used, based on `use_defaults`.
+   * Default prioritization: primary_button, secondary_button, additional_button
+   *   (excluding pseudo-links), dismiss_button, <input>, any button.
+   * @property {Boolean} [use_defaults] Whether to use the default element
+   *   prioritization. If `selector` is provided and the element can't be found,
+   *   and this is set to false, nothing will be selected. If `selector` is not
+   *   provided, this must be true. Defaults to true.
+   */
+
   /**
    * @typedef {Object} Anchor
    * @property {String} selector CSS selector for the anchor node.
@@ -660,6 +661,9 @@ export class FeatureCallout {
    *   [open] style. Buttons do, for example. It's usually similar to :active.
    * @property {Number} [arrow_width] The desired width of the arrow in a number
    *   of pixels. 33.94113 by default (this corresponds to 24px edges).
+   * @property {AutoFocusOptions} [autofocus] Options for the optional autofocus
+   *   feature. Typically omitted, but if provided, an element inside the
+   *   callout will be automatically focused when the callout appears.
    */
 
   /**
@@ -1581,7 +1585,9 @@ export class FeatureCallout {
   }
 
   _dismiss() {
-    let action = this.currentScreen?.content.dismiss_button?.action;
+    let action =
+      this.currentScreen?.content.dismiss_action ??
+      this.currentScreen?.content.dismiss_button?.action;
     if (action?.type) {
       this.win.AWSendToParent("SPECIAL_ACTION", action);
       if (!action.dismiss) {
@@ -1899,33 +1905,43 @@ export class FeatureCallout {
   }
 
   /**
-   * Get the element that should be initially focused. Prioritize the primary
+   * Get the element that should be autofocused when the callout first opens. By
-   * button, then the secondary button, then any additional button, excluding
+   * default, prioritize the primary button, then the secondary button, then any
-   * pseudo-links and the dismiss button. If no button is found, focus the first
+   * additional button, excluding pseudo-links and the dismiss button. If no
-   * input element. If no affirmative action is found, focus the first button,
+   * button is found, focus the first input element. If no affirmative action is
-   * which is probably the dismiss button. If no button is found, focus the
+   * found, focus the first button, which is probably the dismiss button. A
-   * container itself.
+   * custom selector can also be provided to focus a specific element.
+   * @param {AutoFocusOptions} [options]
    * @returns {Element|null} The element to focus when the callout is shown.
    */
-  getInitialFocus() {
+  getAutoFocusElement({ selector, use_defaults = true } = {}) {
     if (!this._container) {
       return null;
     }
-    return (
+    if (selector) {
-      this._container.querySelector(
+      let element = this._container.querySelector(selector);
-        ".primary:not(:disabled, [hidden], .text-link, .cta-link, .split-button)"
+      if (element) {
-      ) ||
+        return element;
-      this._container.querySelector(
+      }
-        ".secondary:not(:disabled, [hidden], .text-link, .cta-link, .split-button)"
+    }
-      ) ||
+    if (use_defaults) {
-      this._container.querySelector(
+      return (
-        "button:not(:disabled, [hidden], .text-link, .cta-link, .dismiss-button, .split-button)"
+        this._container.querySelector(
-      ) ||
+          ".primary:not(:disabled, [hidden], .text-link, .cta-link, .split-button)"
-      this._container.querySelector("input:not(:disabled, [hidden])") ||
+        ) ||
-      this._container.querySelector(
+        this._container.querySelector(
-        "button:not(:disabled, [hidden], .text-link, .cta-link)"
+          ".secondary:not(:disabled, [hidden], .text-link, .cta-link, .split-button)"
-      )
+        ) ||
-    );
+        this._container.querySelector(
+          "button:not(:disabled, [hidden], .text-link, .cta-link, .dismiss-button, .split-button)"
+        ) ||
+        this._container.querySelector("input:not(:disabled, [hidden])") ||
+        this._container.querySelector(
+          "button:not(:disabled, [hidden], .text-link, .cta-link)"
+        )
+      );
+    }
+    return null;
   }
 
   /**
@@ -1945,13 +1961,16 @@ export class FeatureCallout {
       this.renderObserver = new this.win.MutationObserver(() => {
         // Check if the Feature Callout screen has loaded for the first time
         if (!this.ready && this._container.querySelector(".screen")) {
+          const anchor = this._getAnchor();
           const onRender = () => {
             this.ready = true;
             this._pageEventManager?.clear();
             this._attachPageEventListeners(
               this.currentScreen?.content?.page_event_listeners
             );
-            this.getInitialFocus()?.focus();
+            if (anchor?.autofocus) {
+              this.getAutoFocusElement(anchor.autofocus)?.focus();
+            }
             this.win.addEventListener("keypress", this, { capture: true });
             if (this._container.localName === "div") {
               this.win.addEventListener("focus", this, {
@@ -1982,7 +2001,6 @@ export class FeatureCallout {
               this.win.requestAnimationFrame(onRender);
             });
           } else if (this._container.localName === "panel") {
-            const anchor = this._getAnchor();
             if (!anchor?.panel_position) {
               this.endTour();
               return;

browser/components/asrouter/tests/browser/browser_feature_callout_in_chrome.js

@@ -1123,70 +1123,6 @@ add_task(
   }
 );
 
-add_task(async function feature_callout_dismissed_on_escape() {
-  const sandbox = sinon.createSandbox();
-  const sendTriggerStub = sandbox.stub(ASRouter, "sendTriggerMessage");
-  sendTriggerStub.withArgs(pdfMatch).resolves(testMessage);
-  sendTriggerStub.callThrough();
-
-  const win = await BrowserTestUtils.openNewBrowserWindow();
-  await openURLInWindow(win, PDF_TEST_URL);
-  const doc = win.document;
-  await waitForCalloutScreen(doc, testMessageScreenId);
-  const container = doc.querySelector(calloutSelector);
-  ok(
-    container,
-    "Feature Callout is rendered in the browser chrome with a new window when a message is available"
-  );
-
-  // Ensure the browser is focused
-  win.gBrowser.selectedBrowser.focus();
-
-  // Press Escape to close
-  EventUtils.synthesizeKey("KEY_Escape", {}, win);
-  await waitForCalloutRemoved(doc);
-  ok(true, "Feature callout dismissed after pressing Escape");
-
-  await BrowserTestUtils.closeWindow(win);
-  sandbox.restore();
-});
-
-add_task(
-  async function feature_callout_not_dismissed_on_escape_with_interactive_elm_focused() {
-    const sandbox = sinon.createSandbox();
-    const sendTriggerStub = sandbox.stub(ASRouter, "sendTriggerMessage");
-    sendTriggerStub.withArgs(pdfMatch).resolves(testMessage);
-    sendTriggerStub.callThrough();
-
-    const win = await BrowserTestUtils.openNewBrowserWindow();
-    await openURLInWindow(win, PDF_TEST_URL);
-    const doc = win.document;
-    await waitForCalloutScreen(doc, testMessageScreenId);
-    const container = doc.querySelector(calloutSelector);
-    ok(
-      container,
-      "Feature Callout is rendered in the browser chrome with a new window when a message is available"
-    );
-
-    // Ensure an interactive element is focused
-    win.gURLBar.focus();
-
-    // Press Escape to close
-    EventUtils.synthesizeKey("KEY_Escape", {}, win);
-    await TestUtils.waitForTick();
-    // Wait 500ms for transition to complete
-    // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
-    await new Promise(resolve => setTimeout(resolve, 500));
-    ok(
-      doc.querySelector(calloutSelector),
-      "Feature callout is not dismissed after pressing Escape because an interactive element is focused"
-    );
-
-    await BrowserTestUtils.closeWindow(win);
-    sandbox.restore();
-  }
-);
-
 add_task(async function first_anchor_selected_is_valid() {
   const win = await BrowserTestUtils.openNewBrowserWindow();
   const config = {

browser/components/asrouter/tests/browser/browser_feature_callout_panel.js

@@ -354,6 +354,65 @@ add_task(async function feature_callout_split_dismiss_button() {
   );
 });
 
+// Test that the usual focus behavior works: focus remains where it is when the
+// callout opens, and F6 must be pressed to focus the callout.
+add_task(async function feature_callout_no_autofocus() {
+  requestLongerTimeout(2);
+  let message = getTestMessage();
+  const win = await BrowserTestUtils.openNewBrowserWindow();
+  const doc = win.document;
+  const browser = win.gBrowser.selectedBrowser;
+
+  win.focus();
+  win.gURLBar.blur();
+  let onFocused = BrowserTestUtils.waitForEvent(
+    win.gURLBar.inputField,
+    "focus"
+  );
+  win.gURLBar.focus();
+  await onFocused;
+  is(doc.activeElement, win.gURLBar.inputField, "URL bar should be focused");
+
+  let focusedElement = doc.activeElement;
+
+  let popupShown = BrowserTestUtils.waitForEvent(doc, "popupshown", true);
+  let calloutShown = waitForCalloutScreen(doc, message.content.screens[0].id);
+  const { featureCallout, showing, closed } = await showFeatureCallout(
+    browser,
+    message
+  );
+
+  await Promise.all([popupShown, calloutShown]);
+  let calloutContainer = featureCallout._container;
+  ok(showing && calloutContainer, "Feature callout should be showing");
+
+  is(
+    doc.activeElement,
+    focusedElement,
+    "Focus should not change when the callout is shown"
+  );
+
+  let dismissButton = doc.querySelector(calloutDismissSelector);
+  ok(dismissButton, "Callout should have a dismiss button");
+  let onFocused2 = BrowserTestUtils.waitForEvent(dismissButton, "focus", true);
+  EventUtils.synthesizeKey("KEY_F6", {}, win);
+  await onFocused2;
+  is(
+    doc.activeElement,
+    dismissButton,
+    "Callout dismiss button should be focused after F6"
+  );
+
+  EventUtils.synthesizeKey("VK_SPACE", {}, win);
+  await closed;
+  await waitForCalloutRemoved(doc);
+  ok(!doc.querySelector(calloutSelector), "Feature callout should be hidden");
+
+  await BrowserTestUtils.closeWindow(win);
+});
+
+// Test that the autofocus property causes the callout to be focused when shown,
+// and that Tab and Shift+Tab cycle through elements as expected.
 add_task(async function feature_callout_tab_order() {
   let message = getTestMessage();
   message.content.screens[0].content.secondary_button = {
@@ -364,6 +423,8 @@ add_task(async function feature_callout_tab_order() {
     label: { raw: "Advance" },
     action: { navigate: true },
   };
+  // enable autofocus on the anchor
+  message.content.screens[0].anchors[0].autofocus = {};
 
   await testCalloutHiddenIf(
     async (win, calloutContainer) => {