fix(locale): Only use the users preferred locale to check for cms customization

Author: vbudhram

packages/fxa-settings/src/lib/config.ts

@@ -101,6 +101,7 @@ export interface Config {
   nimbusPreview: boolean;
   cms: {
     enabled: boolean;
+    l10nEnabled: boolean;
   };
 }
 
@@ -185,8 +186,8 @@ export function getDefault() {
       showLocaleToggle: false,
     },
     cms: {
-      // Note: Even if this flag is true, the user must be an `en` language
       enabled: false,
+      l10nEnabled: false
     },
     nimbusPreview: false,
   } as Config;

packages/fxa-settings/src/models/hooks.test.ts

@@ -7,7 +7,9 @@ import { renderHook, waitFor } from '@testing-library/react';
 import { ReactNode, createElement } from 'react';
 import * as Sentry from '@sentry/browser';
 
-import { useCmsInfoState } from './hooks';
+import {
+  useCmsInfoState
+} from './hooks';
 import { AppContext } from './contexts/AppContext';
 
 // Mock all external dependencies before importing the hook
@@ -45,6 +47,10 @@ jest.mock('../lib/integrations', () => ({
 
 jest.mock('../lib/nimbus', () => ({}));
 
+jest.mock('../contexts/DynamicLocalizationContext', () => ({
+  useDynamicLocalization: jest.fn().mockReturnValue({ currentLocale: 'en-US' }),
+}));
+
 // Mock fetch
 const mockFetch = jest.fn();
 global.fetch = mockFetch;
@@ -93,6 +99,17 @@ describe('useCmsInfoState', () => {
       ok: true,
       json: jest.fn().mockResolvedValue({ customization: 'test' }),
     });
+
+    // Reset navigator.language and useDynamicLocalization to English defaults
+    Object.defineProperty(navigator, 'language', {
+      writable: true,
+      value: 'en-US',
+    });
+    Object.defineProperty(navigator, 'languages', {
+      writable: true,
+      value: ['en-US', 'en'],
+    });
+    require('../contexts/DynamicLocalizationContext').useDynamicLocalization.mockReturnValue({ currentLocale: 'en-US' });
   });
 
   it('should use default entrypoint when entrypoint is not provided in URL', async () => {
@@ -239,6 +256,9 @@ describe('useCmsInfoState', () => {
       value: ['fr-FR', 'fr'],
     });
 
+    // Mock useDynamicLocalization to return non-English locale
+    require('../contexts/DynamicLocalizationContext').useDynamicLocalization.mockReturnValue({ currentLocale: 'fr-FR' });
+
     mockUrlQueryData.get.mockImplementation((key: string) => {
       if (key === 'client_id') return '1234567890abcdef';
       if (key === 'entrypoint') return 'preferences';
@@ -249,21 +269,15 @@ describe('useCmsInfoState', () => {
       wrapper: MockAppProvider,
     });
 
+    // Wait for the effect to complete
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
     // Should not fetch due to non-English locale
     expect(mockFetch).not.toHaveBeenCalled();
-    expect(result.current.loading).toBe(false);
     expect(result.current.data).toBeUndefined();
     expect(result.current.error).toBeUndefined();
-
-    // Reset to English for other tests
-    Object.defineProperty(navigator, 'language', {
-      writable: true,
-      value: 'en-US',
-    });
-    Object.defineProperty(navigator, 'languages', {
-      writable: true,
-      value: ['en-US', 'en'],
-    });
   });
 
   it('should handle fetch errors gracefully', async () => {
@@ -354,47 +368,106 @@ describe('useCmsInfoState', () => {
     );
   });
 
-  it('should handle various custom entrypoint values', async () => {
-    const testCases = [
-      'firefox-toolbar',
-      'fxa_app_menu',
-      'ios_settings_manage',
-      'synced-tabs',
-      'custom.entrypoint-123'
-    ];
-
-    for (const entrypoint of testCases) {
-      jest.clearAllMocks();
-
-      mockUrlQueryData.get.mockImplementation((key: string) => {
-        if (key === 'client_id') return '1234567890abcdef';
-        if (key === 'entrypoint') return entrypoint;
-        return null;
-      });
-
-      mockFetch.mockResolvedValue({
-        ok: true,
-        json: jest.fn().mockResolvedValue({ customization: `test-${entrypoint}` }),
-      });
-
-      const { result } = renderHook(() => useCmsInfoState(), {
-        wrapper: MockAppProvider,
-      });
-
-      // Wait for the fetch to complete
-      await waitFor(() => {
-        expect(result.current.loading).toBe(false);
-      });
-
-      // Verify that fetch was called with the correct entrypoint
-      expect(mockFetch).toHaveBeenCalledWith(
-        expect.objectContaining({
-          href: `http://localhost:9000/v1/cms/config?clientId=1234567890abcdef&entrypoint=${entrypoint}`,
-        }),
-        expect.any(Object)
-      );
-
-      expect(result.current.data?.cmsInfo).toEqual({ customization: `test-${entrypoint}` });
-    }
+  it('should fetch when l10nEnabled is true for any locale', async () => {
+    const l10nEnabledConfig = {
+      ...mockConfig,
+      cms: { enabled: true, l10nEnabled: true },
+    };
+
+    const L10nEnabledProvider = ({ children }: { children: ReactNode }) =>
+      createElement(AppContext.Provider, {
+        value: { config: l10nEnabledConfig as any, account: undefined, apolloClient: undefined }
+      }, children);
+
+    // Mock non-English locale
+    require('../contexts/DynamicLocalizationContext').useDynamicLocalization.mockReturnValue({ currentLocale: 'fr-FR' });
+    Object.defineProperty(navigator, 'language', {
+      writable: true,
+      value: 'fr-FR',
+    });
+
+    mockUrlQueryData.get.mockImplementation((key: string) => {
+      if (key === 'client_id') return '1234567890abcdef';
+      if (key === 'entrypoint') return 'preferences';
+      return null;
+    });
+
+    mockFetch.mockResolvedValue({
+      ok: true,
+      json: jest.fn().mockResolvedValue({ customization: 'french-test' }),
+    });
+
+    const { result } = renderHook(() => useCmsInfoState(), {
+      wrapper: L10nEnabledProvider,
+    });
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
+    expect(mockFetch).toHaveBeenCalled();
+    expect(result.current.data?.cmsInfo).toEqual({ customization: 'french-test' });
+  });
+
+  it('should fetch when l10nEnabled is false but browser language is English', async () => {
+    // Mock English browser language but non-English selected locale
+    Object.defineProperty(navigator, 'language', {
+      writable: true,
+      value: 'en-GB',
+    });
+    require('../contexts/DynamicLocalizationContext').useDynamicLocalization.mockReturnValue({ currentLocale: 'fr-FR' });
+
+    mockUrlQueryData.get.mockImplementation((key: string) => {
+      if (key === 'client_id') return '1234567890abcdef';
+      if (key === 'entrypoint') return 'preferences';
+      return null;
+    });
+
+    mockFetch.mockResolvedValue({
+      ok: true,
+      json: jest.fn().mockResolvedValue({ customization: 'english-browser-test' }),
+    });
+
+    const { result } = renderHook(() => useCmsInfoState(), {
+      wrapper: MockAppProvider,
+    });
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
+    expect(mockFetch).toHaveBeenCalled();
+    expect(result.current.data?.cmsInfo).toEqual({ customization: 'english-browser-test' });
+  });
+
+  it('should fetch when l10nEnabled is false but selected locale is English', async () => {
+    // Mock non-English browser language but English selected locale
+    Object.defineProperty(navigator, 'language', {
+      writable: true,
+      value: 'fr-FR',
+    });
+    require('../contexts/DynamicLocalizationContext').useDynamicLocalization.mockReturnValue({ currentLocale: 'en-US' });
+
+    mockUrlQueryData.get.mockImplementation((key: string) => {
+      if (key === 'client_id') return '1234567890abcdef';
+      if (key === 'entrypoint') return 'preferences';
+      return null;
+    });
+
+    mockFetch.mockResolvedValue({
+      ok: true,
+      json: jest.fn().mockResolvedValue({ customization: 'english-selected-test' }),
+    });
+
+    const { result } = renderHook(() => useCmsInfoState(), {
+      wrapper: MockAppProvider,
+    });
+
+    await waitFor(() => {
+      expect(result.current.loading).toBe(false);
+    });
+
+    expect(mockFetch).toHaveBeenCalled();
+    expect(result.current.data?.cmsInfo).toEqual({ customization: 'english-selected-test' });
   });
 });

packages/fxa-settings/src/models/hooks.ts

@@ -230,23 +230,29 @@ export function useCmsInfoState() {
   useEffect(() => {
     // We disable the CMS if:
     // 1. CMS is not enabled in the config
-    // 2. The user's locale is not English
-    // 3. The clientId is not provided or is not a valid 16 digit hex
-
-    function isEnglishLocale() {
-      // Check primary language
-      const primaryLanguage = navigator.language;
-      if (primaryLanguage.startsWith('en')) {
+    // 2. l10nEnabled is false AND user's locale is not English
+    // 3. User language toggle is not English
+    // 4. The clientId is not provided or is not a valid 16 digit hex
+    //
+    // These steps are a best effort to prevent users from seeing a page with both English and non-English text.
+
+    function shouldFetchCms() {
+      // If l10nEnabled is true, fetch cms config for any locale
+      if (config?.cms?.l10nEnabled === true) {
         return true;
       }
 
-      // Check preferred languages
-      return navigator.languages.some(lang => lang.startsWith('en'));
+      // If l10nEnabled is false, only fetch for English locales
+      // Check both browser language and user's selected locale
+      const isEnglishBrowser = navigator.language.startsWith('en');
+      const isEnglishSelected = currentLocale.startsWith('en');
+
+      return isEnglishBrowser || isEnglishSelected;
     }
 
     if (
       !config.cms.enabled ||
-      !isEnglishLocale() ||
+      !shouldFetchCms() ||
       !clientId ||
       !isHexadecimal(clientId) ||
       !length(clientId, 16) ||
@@ -306,7 +312,7 @@ export function useCmsInfoState() {
     return () => {
       mounted = false;
     };
-  }, [authUrl, clientId, entrypoint, config.cms.enabled, currentLocale]);
+  }, [authUrl, clientId, entrypoint, config.cms.enabled, config.cms?.l10nEnabled, currentLocale]);
 
   return state;
 }