Skip to content

Commit a4882ea

Browse files
authored
Merge pull request #19793 from mozilla/FXA-12760
fix(sync): Only send fxaStatus if UA is likely Fx, update unsupported context list
2 parents 5cb65a4 + 4eedb67 commit a4882ea

5 files changed

Lines changed: 42 additions & 8 deletions

File tree

packages/fxa-settings/src/components/App/index.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
useIntegration,
3131
useLocalSignedInQueryState,
3232
useSession,
33+
isProbablyFirefox,
3334
} from '../../models';
3435
import {
3536
initializeSettingsContext,
@@ -216,12 +217,8 @@ export const App = ({
216217
// Request and update account data/state to match the browser state.
217218
// If there is a user actively signed into the browser,
218219
// we should try to use that user's account when possible.
219-
const ua = navigator.userAgent.toLowerCase();
220-
// This may not catch all Firefox browsers notably iOS devices, see FXA-11520 for alternate approach
221-
const isProbablyFirefox = ua.includes('firefox') || ua.includes('fxios');
222-
223220
let userFromBrowser;
224-
if (isProbablyFirefox) {
221+
if (isProbablyFirefox()) {
225222
userFromBrowser = await firefox.requestSignedInUser(
226223
integration.data.context || '',
227224
// TODO with React pairing flow, update this if pairing flow

packages/fxa-settings/src/lib/hooks/useFxAStatus/index.test.tsx

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { renderHook } from '@testing-library/react-hooks';
66
import { useFxAStatus } from '.';
77
import { Constants } from '../../constants';
88
import firefox from '../../channels/firefox';
9-
import { IntegrationType } from '../../../models';
9+
import { IntegrationType, isProbablyFirefox } from '../../../models';
1010

1111
jest.mock('../../channels/firefox', () => ({
1212
__esModule: true,
@@ -15,9 +15,18 @@ jest.mock('../../channels/firefox', () => ({
1515
},
1616
}));
1717

18+
jest.mock('../../../models', () => {
19+
const actual = jest.requireActual('../../../models');
20+
return {
21+
...actual,
22+
isProbablyFirefox: jest.fn(() => true),
23+
};
24+
});
25+
1826
describe('useFxAStatus', () => {
1927
beforeEach(() => {
20-
jest.resetAllMocks();
28+
jest.clearAllMocks();
29+
(isProbablyFirefox as jest.Mock).mockImplementation(() => true);
2130
});
2231

2332
describe('SyncDesktopV3 integration', () => {
@@ -149,4 +158,19 @@ describe('useFxAStatus', () => {
149158
expect(firefox.fxaStatus).not.toHaveBeenCalled();
150159
});
151160
});
161+
162+
describe('Non-Firefox browser', () => {
163+
it('does not call fxaStatus when isProbablyFirefox returns false', () => {
164+
(isProbablyFirefox as jest.Mock).mockReturnValueOnce(false);
165+
166+
const integration = {
167+
type: IntegrationType.SyncDesktopV3,
168+
isSync: () => true,
169+
isFirefoxNonSync: () => false,
170+
};
171+
172+
renderHook(() => useFxAStatus(integration));
173+
expect(firefox.fxaStatus).not.toHaveBeenCalled();
174+
});
175+
});
152176
});

packages/fxa-settings/src/lib/hooks/useFxAStatus/index.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
isOAuthIntegration,
99
isSyncDesktopV3Integration,
1010
isOAuthNativeIntegration,
11+
isProbablyFirefox,
1112
} from '../../../models';
1213
import {
1314
defaultDesktopV3SyncEngineConfigs,
@@ -45,7 +46,7 @@ export function useFxAStatus(integration: FxAStatusIntegration) {
4546
useEffect(() => {
4647
// This sends a web channel message to the browser to prompt a response
4748
// that we listen for.
48-
if (isSync || isOAuthNative) {
49+
if ((isSync || isOAuthNative) && isProbablyFirefox()) {
4950
(async () => {
5051
const status = await firefox.fxaStatus({
5152
// TODO: Improve getting 'context', probably set this on the integration

packages/fxa-settings/src/models/integrations/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,4 @@ export * from './sync-desktop-v3-integration';
1414
export * from './web-integration';
1515
export * from './third-party-auth-callback-integration';
1616
export * from './relier-interfaces';
17+
export * from './utils';

packages/fxa-settings/src/models/integrations/utils.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export function isFirefoxService(service?: string) {
1212
}
1313

1414
const NO_LONGER_SUPPORTED_CONTEXTS = new Set([
15+
'fx_ios_v1',
1516
'fx_desktop_v1',
1617
'fx_desktop_v2',
1718
'fx_firstrun_v2',
@@ -20,3 +21,13 @@ const NO_LONGER_SUPPORTED_CONTEXTS = new Set([
2021
export function isUnsupportedContext(context?: string): boolean {
2122
return !!context && NO_LONGER_SUPPORTED_CONTEXTS.has(context);
2223
}
24+
25+
/**
26+
* Checks if the browser is probably Firefox based on the user agent string.
27+
* This may not catch all Firefox browsers, notably iOS devices.
28+
* See FXA-11520 for alternate approach.
29+
*/
30+
export function isProbablyFirefox(): boolean {
31+
const ua = navigator.userAgent.toLowerCase();
32+
return ua.includes('firefox') || ua.includes('fxios');
33+
}

0 commit comments

Comments
 (0)