From 1fd932fd30f008f0e1d183d79f11261172005f27 Mon Sep 17 00:00:00 2001
From: MagentaManifold <17zhaomingyuan@gmail.com>
Date: Thu, 26 Feb 2026 14:37:40 -0500
Subject: [PATCH] fix(settings): fix metrics not always emitted for third party
 auth

Because:

* metrics not always emitted for third party auth

This commit:

* avoids using <form> for navigation

Closes FXA-12949
---
 packages/fxa-react/lib/utils.tsx              |   7 +-
 .../__snapshots__/index.test.tsx.snap         |  12 +-
 .../components/ThirdPartyAuth/index.test.tsx  | 107 +++++++---------
 .../src/components/ThirdPartyAuth/index.tsx   | 118 ++++++++----------
 4 files changed, 106 insertions(+), 138 deletions(-)

diff --git a/packages/fxa-react/lib/utils.tsx b/packages/fxa-react/lib/utils.tsx
index 6ffe76177f0..1ba2f137e2d 100644
--- a/packages/fxa-react/lib/utils.tsx
+++ b/packages/fxa-react/lib/utils.tsx
@@ -16,7 +16,8 @@ export function hardNavigate(
   href: string,
   additionalQueryParams: Record<string, string> = {},
   includeCurrentQueryParams = false,
-  replace: boolean = false
+  replace: boolean = false,
+  timeoutMs: number = DEFAULT_NAVIGATE_TIMEOUT_MS
 ) {
   // If there are any query params in the href, we automatically include them in the new url.
   const url = new URL(href, window.location.origin);
@@ -40,11 +41,11 @@ export function hardNavigate(
   if (replace) {
     setTimeout(() => {
       window.location.replace(url.href);
-    }, DEFAULT_NAVIGATE_TIMEOUT_MS);
+    }, timeoutMs);
   } else {
     setTimeout(() => {
       window.location.href = url.href;
-    }, DEFAULT_NAVIGATE_TIMEOUT_MS);
+    }, timeoutMs);
   }
 }
 
diff --git a/packages/fxa-settings/src/components/ThirdPartyAuth/__snapshots__/index.test.tsx.snap b/packages/fxa-settings/src/components/ThirdPartyAuth/__snapshots__/index.test.tsx.snap
index 78acdcd6cfa..19c7da56f2c 100644
--- a/packages/fxa-settings/src/components/ThirdPartyAuth/__snapshots__/index.test.tsx.snap
+++ b/packages/fxa-settings/src/components/ThirdPartyAuth/__snapshots__/index.test.tsx.snap
@@ -4,9 +4,9 @@ exports[`ThirdPartyAuthComponent buttons match snapshot: apple 1`] = `
 <button
   aria-label="Continue with Apple"
   class="w-[60px] h-[60px] p-4 flex items-center justify-center rounded-full border focus-visible-default outline-offset-2
-        bg-black border-transparent
-        "
-  type="submit"
+      bg-black border-transparent
+      "
+  type="button"
 >
   <svg
     class="w-full h-auto"
@@ -20,9 +20,9 @@ exports[`ThirdPartyAuthComponent buttons match snapshot: google 1`] = `
 <button
   aria-label="Continue with Google"
   class="w-[60px] h-[60px] p-4 flex items-center justify-center rounded-full border focus-visible-default outline-offset-2
-        bg-[#F9F4F4] border-[#747775] border-[1px]
-        "
-  type="submit"
+      bg-[#F9F4F4] border-[#747775] border-[1px]
+      "
+  type="button"
 >
   <svg
     class="w-full h-auto"
diff --git a/packages/fxa-settings/src/components/ThirdPartyAuth/index.test.tsx b/packages/fxa-settings/src/components/ThirdPartyAuth/index.test.tsx
index 2f5efc3d147..a5cb4e00130 100644
--- a/packages/fxa-settings/src/components/ThirdPartyAuth/index.test.tsx
+++ b/packages/fxa-settings/src/components/ThirdPartyAuth/index.test.tsx
@@ -3,8 +3,10 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import React from 'react';
-import { screen } from '@testing-library/react';
+import { screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider';
+import * as ReactUtils from 'fxa-react/lib/utils';
 import { Subject } from './mocks';
 
 const mockViewWithNoPasswordSet = jest.fn();
@@ -14,14 +16,14 @@ const mockStartGoogleAuthFromLogin = jest.fn();
 const mockStartAppleAuthFromLogin = jest.fn();
 const mockStartGoogleAuthFromReg = jest.fn();
 const mockStartAppleAuthFromReg = jest.fn();
-const mockGleanIsDone = jest.fn();
+const mockGleanIsDone = jest.fn().mockResolvedValue(undefined);
 
 jest.mock('../../lib/glean', () => {
   return {
     __esModule: true,
     default: {
       isDone: () => {
-        mockGleanIsDone();
+        return mockGleanIsDone();
       },
       emailFirst: {
         googleOauthStart: () => {
@@ -56,22 +58,20 @@ function renderWith(props?: any) {
   return renderWithLocalizationProvider(<Subject {...props} />);
 }
 
-const mockFormSubmit = jest.fn();
-// jsdom does not implement HTMLFormElement.prototype.submit.
-HTMLFormElement.prototype.submit = mockFormSubmit;
-
 describe('ThirdPartyAuthComponent', () => {
-  // Form submission not supported in jest test. Instead, prevent the submission
-  // and illustrate a 'short circuit' operation.
-  const onSubmit = (e: any) => {
-    e.preventDefault();
-    return false;
-  };
-  const onContinueWithApple = jest.fn().mockImplementation(onSubmit);
-  const onContinueWithGoogle = jest.fn().mockImplementation(onSubmit);
+  const onContinueWithApple = jest.fn();
+  const onContinueWithGoogle = jest.fn();
+  let hardNavigateSpy: jest.SpyInstance;
+
+  beforeEach(() => {
+    hardNavigateSpy = jest
+      .spyOn(ReactUtils, 'hardNavigate')
+      .mockImplementation(() => {});
+  });
 
   afterEach(() => {
-    jest.resetAllMocks();
+    hardNavigateSpy.mockRestore();
+    jest.clearAllMocks();
   });
 
   it('renders', async () => {
@@ -84,69 +84,68 @@ describe('ThirdPartyAuthComponent', () => {
 
     await screen.findByLabelText('Continue with Google');
     await screen.findByLabelText('Continue with Apple');
-    expect(mockFormSubmit).not.toHaveBeenCalled();
-
-    expect(
-      (await screen.findByTestId('google-signin-form-state')).getAttribute(
-        'value'
-      )
-    ).toEqual('http%3A%2F%2Flocalhost%2F%3FflowId%3D123');
-
-    expect(
-      (await screen.findByTestId('apple-signin-form-state')).getAttribute(
-        'value'
-      )
-    ).toEqual('http%3A%2F%2Flocalhost%2F%3FflowId%3D123');
   });
 
   it('submits apple form', async () => {
+    const user = userEvent.setup();
     renderWith({
       enabled: true,
       showSeparator: true,
       onContinueWithApple,
       onContinueWithGoogle,
+      flowQueryParams: { flowId: '123' },
     });
 
     const button = await screen.findByLabelText('Continue with Apple');
-    button.click();
+    await user.click(button);
 
-    // Ensure the hidden input for state was updated.
-    expect(
-      (await screen.findByTestId('apple-signin-form-state')).getAttribute(
-        'value'
-      )
-    ).not.toEqual('');
     expect(onContinueWithApple).toHaveBeenCalled();
     expect(onContinueWithGoogle).not.toHaveBeenCalled();
+    await waitFor(() => {
+      expect(hardNavigateSpy).toHaveBeenCalledWith(
+        expect.any(String),
+        expect.objectContaining({
+          state: 'http%3A%2F%2Flocalhost%2F%3FflowId%3D123',
+        }),
+        false,
+        false,
+        0
+      );
+    });
   });
 
   it('submits google form', async () => {
+    const user = userEvent.setup();
     renderWith({
       enabled: true,
       showSeparator: true,
       onContinueWithApple,
       onContinueWithGoogle,
+      flowQueryParams: { flowId: '123' },
     });
 
     const button = await screen.findByLabelText('Continue with Google');
-    button.click();
+    await user.click(button);
 
-    // Ensure the hidden input for state was updated.
-    expect(
-      (await screen.findByTestId('google-signin-form-state')).getAttribute(
-        'value'
-      )
-    ).not.toEqual('');
     expect(onContinueWithGoogle).toHaveBeenCalled();
     expect(onContinueWithApple).not.toHaveBeenCalled();
+    await waitFor(() => {
+      expect(hardNavigateSpy).toHaveBeenCalledWith(
+        expect.any(String),
+        expect.objectContaining({
+          state: 'http%3A%2F%2Flocalhost%2F%3FflowId%3D123',
+        }),
+        false,
+        false,
+        0
+      );
+    });
   });
 
   it('hides separator', async () => {
     renderWith({
       enabled: true,
       showSeparator: false,
-      onContinueWithApple,
-      onContinueWithGoogle,
     });
 
     expect(screen.queryByText('Or')).toBeNull();
@@ -157,8 +156,6 @@ describe('ThirdPartyAuthComponent', () => {
     renderWith({
       enabled: true,
       showSeparator: true,
-      onContinueWithApple,
-      onContinueWithGoogle,
     });
 
     expect(screen.queryByText('Or')).toBeDefined();
@@ -168,8 +165,6 @@ describe('ThirdPartyAuthComponent', () => {
   it('buttons match snapshot', async () => {
     renderWith({
       enabled: true,
-      onContinueWithApple,
-      onContinueWithGoogle,
     });
 
     const googleButton = await screen.findByLabelText('Continue with Google');
@@ -183,8 +178,6 @@ describe('ThirdPartyAuthComponent', () => {
       renderWith({
         enabled: true,
         showSeparator: false,
-        onContinueWithApple,
-        onContinueWithGoogle,
       });
       expect(mockViewWithNoPasswordSet).toHaveBeenCalled();
     });
@@ -193,8 +186,6 @@ describe('ThirdPartyAuthComponent', () => {
       renderWith({
         enabled: true,
         showSeparator: false,
-        onContinueWithApple,
-        onContinueWithGoogle,
         viewName: 'index',
       });
       const button = await screen.findByLabelText('Continue with Google');
@@ -207,8 +198,6 @@ describe('ThirdPartyAuthComponent', () => {
       renderWith({
         enabled: true,
         showSeparator: false,
-        onContinueWithApple,
-        onContinueWithGoogle,
         viewName: 'index',
       });
       const button = await screen.findByLabelText('Continue with Apple');
@@ -220,8 +209,6 @@ describe('ThirdPartyAuthComponent', () => {
       renderWith({
         enabled: true,
         showSeparator: false,
-        onContinueWithApple,
-        onContinueWithGoogle,
         viewName: 'signin',
       });
       const button = await screen.findByLabelText('Continue with Google');
@@ -234,8 +221,6 @@ describe('ThirdPartyAuthComponent', () => {
       renderWith({
         enabled: true,
         showSeparator: false,
-        onContinueWithApple,
-        onContinueWithGoogle,
         viewName: 'signin',
       });
       const button = await screen.findByLabelText('Continue with Apple');
@@ -247,8 +232,6 @@ describe('ThirdPartyAuthComponent', () => {
       renderWith({
         enabled: true,
         showSeparator: false,
-        onContinueWithApple,
-        onContinueWithGoogle,
         viewName: 'signup',
       });
       const button = await screen.findByLabelText('Continue with Google');
@@ -261,8 +244,6 @@ describe('ThirdPartyAuthComponent', () => {
       renderWith({
         enabled: true,
         showSeparator: false,
-        onContinueWithApple,
-        onContinueWithGoogle,
         viewName: 'signup',
       });
       const button = await screen.findByLabelText('Continue with Apple');
diff --git a/packages/fxa-settings/src/components/ThirdPartyAuth/index.tsx b/packages/fxa-settings/src/components/ThirdPartyAuth/index.tsx
index 5e1903c81cc..183e853316f 100644
--- a/packages/fxa-settings/src/components/ThirdPartyAuth/index.tsx
+++ b/packages/fxa-settings/src/components/ThirdPartyAuth/index.tsx
@@ -2,8 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import React, { useRef, FormEventHandler, useEffect, useCallback } from 'react';
-import { FtlMsg } from 'fxa-react/lib/utils';
+import React, { useEffect, useCallback } from 'react';
+import { FtlMsg, hardNavigate } from 'fxa-react/lib/utils';
 import { useLocalization } from '@fluent/react';
 
 import { ReactComponent as GoogleLogo } from './google-logo-viewbox.svg';
@@ -16,8 +16,8 @@ import GleanMetrics from '../../lib/glean';
 import { QueryParams } from '../..';
 
 export type ThirdPartyAuthProps = {
-  onContinueWithGoogle?: FormEventHandler<HTMLFormElement>;
-  onContinueWithApple?: FormEventHandler<HTMLFormElement>;
+  onContinueWithGoogle?: () => void;
+  onContinueWithApple?: () => void;
   showSeparator?: boolean;
   viewName?: string;
   flowQueryParams?: QueryParams;
@@ -73,16 +73,16 @@ const ThirdPartyAuth = ({
           </>
         )}
         <div className="flex gap-4 justify-center">
-          <ThirdPartySignInForm
+          <ThirdPartySignInButton
             {...{
               party: 'google',
               ...config.googleAuthConfig,
-              state: getState(flowQueryParams),
               scope: 'openid email profile',
               responseType: 'code',
               accessType: 'offline',
               prompt: 'consent',
               viewName,
+              flowQueryParams,
               onSubmit: onContinueWithGoogle,
               buttonText: (
                 <>
@@ -91,17 +91,17 @@ const ThirdPartyAuth = ({
               ),
             }}
           />
-          <ThirdPartySignInForm
+          <ThirdPartySignInButton
             {...{
               party: 'apple',
               ...config.appleAuthConfig,
-              state: getState(flowQueryParams),
               scope: 'email',
               responseType: 'code id_token',
               accessType: 'offline',
               prompt: 'consent',
               viewName,
               responseMode: 'form_post',
+              flowQueryParams,
               onSubmit: onContinueWithApple,
               buttonText: (
                 <>
@@ -116,15 +116,9 @@ const ThirdPartyAuth = ({
   );
 };
 
-/**
- * Represents the sign in form posted to google third party auth.
- * Note that the innerRef is used by the parent component to trigger
- * a form submission.
- */
-const ThirdPartySignInForm = ({
+const ThirdPartySignInButton = ({
   party,
   authorizationEndpoint,
-  state,
   clientId,
   scope,
   redirectUri,
@@ -139,7 +133,6 @@ const ThirdPartySignInForm = ({
 }: {
   party: 'google' | 'apple';
   authorizationEndpoint: string;
-  state: string;
   clientId: string;
   scope: string;
   redirectUri: string;
@@ -148,14 +141,12 @@ const ThirdPartySignInForm = ({
   responseType: string;
   responseMode?: string;
   buttonText: ReactElement;
-  onSubmit?: FormEventHandler<HTMLFormElement>;
+  onSubmit?: () => void;
   viewName?: string;
   flowQueryParams?: QueryParams;
 }) => {
   const { logViewEventOnce } = useMetrics();
   const { l10n } = useLocalization();
-  const stateRef = useRef<HTMLInputElement>(null);
-  const formRef = useRef<HTMLFormElement>(null);
 
   const getLoginAriaLabel = () => {
     const labels = {
@@ -173,7 +164,8 @@ const ThirdPartySignInForm = ({
     return labels[party];
   };
 
-  const onClick = useCallback(async () => {
+  const handleClick = useCallback(async () => {
+    onSubmit?.();
     logViewEventOnce(`flow.${party}`, 'oauth-start');
     switch (`${party}-${viewName}`) {
       case 'google-index':
@@ -196,57 +188,51 @@ const ThirdPartySignInForm = ({
         break;
     }
 
-    // wait for all the Glean events to be sent before the page unloads
+    // Wait for all Glean events to be sent before navigating away.
     await GleanMetrics.isDone();
 
-    if (stateRef.current) {
-      stateRef.current.value = getState(flowQueryParams);
-    }
-  }, [party, stateRef, viewName, logViewEventOnce, flowQueryParams]);
-
-  if (onSubmit === undefined) {
-    onSubmit = () => true;
-  }
+    const params = {
+      state: getState(flowQueryParams),
+      client_id: clientId,
+      scope,
+      redirect_uri: redirectUri,
+      access_type: accessType,
+      prompt,
+      response_type: responseType,
+      ...(responseMode ? { response_mode: responseMode } : {}),
+    };
+    hardNavigate(authorizationEndpoint, params, false, false, 0);
+  }, [
+    party,
+    viewName,
+    logViewEventOnce,
+    flowQueryParams,
+    clientId,
+    scope,
+    redirectUri,
+    accessType,
+    prompt,
+    responseType,
+    responseMode,
+    authorizationEndpoint,
+    onSubmit,
+  ]);
 
   return (
-    <form
-      action={authorizationEndpoint}
-      method="GET"
-      onSubmit={onSubmit}
-      ref={formRef}
+    <button
+      type="button"
+      className={`w-[60px] h-[60px] p-4 flex items-center justify-center rounded-full border focus-visible-default outline-offset-2
+      ${
+        party === 'google'
+          ? 'bg-[#F9F4F4] border-[#747775] border-[1px]'
+          : 'bg-black border-transparent'
+      }
+      `}
+      onClick={handleClick}
+      aria-label={getLoginAriaLabel()}
     >
-      <input
-        data-testid={`${party}-signin-form-state`}
-        ref={stateRef}
-        type="hidden"
-        name="state"
-        value={state}
-      />
-      <input type="hidden" name="client_id" value={clientId} />
-      <input type="hidden" name="scope" value={scope} />
-      <input type="hidden" name="redirect_uri" value={redirectUri} />
-      <input type="hidden" name="access_type" value={accessType} />
-      <input type="hidden" name="prompt" value={prompt} />
-      <input type="hidden" name="response_type" value={responseType} />
-      {responseMode && (
-        <input type="hidden" name="response_mode" value={responseMode} />
-      )}
-
-      <button
-        type="submit"
-        className={`w-[60px] h-[60px] p-4 flex items-center justify-center rounded-full border focus-visible-default outline-offset-2
-        ${
-          party === 'google'
-            ? 'bg-[#F9F4F4] border-[#747775] border-[1px]'
-            : 'bg-black border-transparent'
-        }
-        `}
-        onClick={onClick}
-        aria-label={getLoginAriaLabel()}
-      >
-        {buttonText}
-      </button>
-    </form>
+      {buttonText}
+    </button>
   );
 };