Skip to content

Commit 5406044

Browse files
authored
fix: Links Redirected to Chrome App Bug (Android bug) (MetaMask#25969)
## **Description** On Android, tapping a `target="_blank"` link (e.g. the "Stake" button on lido.fi) in the in-app browser opens the URL in Chrome instead of loading it within the MetaMask browser. This does not happen on iOS. **Root cause:** The `@metamask/react-native-webview` library handles `target="_blank"` differently per platform when the `onOpenWindow` prop is not provided: - **iOS** has a built-in fallback in `createWebViewWithConfiguration` that loads the URL in the same WKWebView. - **Android** has no such fallback. With `setSupportMultipleWindows` defaulting to `true`, the native `RNCWebChromeClient.onCreateWindow` creates a bare `WebView` with no custom `WebViewClient`. Since `mHasOnOpenWindowEvent` is `false`, the URL is loaded in this bare WebView which has no UI container, causing Android to delegate to the system browser (Chrome). **Fix:** Added an `onOpenWindow` handler to the WebView in `BrowserTab` that intercepts `target="_blank"` navigations and loads the URL in the current tab via `window.location.href`. This matches the existing iOS fallback behavior and makes both platforms consistent. The handler reuses the same `sanitizeUrlInput` + `injectJavaScript` pattern already used elsewhere in the file for deeplink browser callbacks. ## **Changelog** CHANGELOG entry: Fixed an Android-only bug where `target="_blank"` links in the in-app browser opened in Chrome instead of loading within MetaMask ## **Related issues** Fixes: <!-- Add issue number if applicable --> Jira Ticket: https://consensyssoftware.atlassian.net/browse/MCWP-321 ## **Manual testing steps** ```gherkin Feature: In-app browser target="_blank" link handling Scenario: user taps a target="_blank" link on Android Given the user has the MetaMask app open on Android And the user navigates to https://lido.fi in the in-app browser When user taps the "Stake" button (which is an <a target="_blank"> link to https://stake.lido.fi) Then the URL loads within the MetaMask in-app browser And the user is NOT redirected to Chrome or any external browser Scenario: user taps a target="_blank" link on iOS Given the user has the MetaMask app open on iOS And the user navigates to https://lido.fi in the in-app browser When user taps the "Stake" button (which is an <a target="_blank"> link to https://stake.lido.fi) Then the URL loads within the MetaMask in-app browser (same behavior as before the fix) Scenario: user taps a regular link (no target="_blank") Given the user has the MetaMask app open on either platform And the user navigates to any dApp in the in-app browser When user taps a regular link without target="_blank" Then the link navigates normally within the in-app browser (no regression) ``` ## **Screenshots/Recordings** ### **Before** <!-- [screenshots/recordings showing Chrome opening on Android when tapping target="_blank" link] --> ### **After** <!-- [screenshots/recordings showing URL loading within in-app browser on Android] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Localized to in-app browser WebView navigation; behavior change is straightforward and covered by new unit tests, with minimal impact outside `target="_blank"` handling. > > **Overview** > Fixes Android in-app browser behavior where `target="_blank"` links / `window.open()` could escape to the system browser by adding an explicit `onOpenWindow` handler that redirects the current WebView via injected `window.location.href` (with `sanitizeUrlInput`). > > Updates tests to mock the WebView ref `injectJavaScript`, adds coverage for `onOpenWindow` behavior (including quote sanitization and empty URL), and refreshes the BrowserTab snapshot to include the new prop. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 744973c. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 8fad90c commit 5406044

3 files changed

Lines changed: 122 additions & 0 deletions

File tree

app/components/Views/BrowserTab/BrowserTab.tsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import React, {
88
import { View, Alert, BackHandler, ImageSourcePropType } from 'react-native';
99
import { isEqual } from 'lodash';
1010
import { WebView, WebViewMessageEvent } from '@metamask/react-native-webview';
11+
import type { WebViewOpenWindowEvent } from '@metamask/react-native-webview/lib/WebViewTypes';
1112
import { useSharedValue } from 'react-native-reanimated';
1213
import BrowserBottomBar from '../../UI/BrowserBottomBar';
1314
import { connect, useSelector } from 'react-redux';
@@ -751,6 +752,22 @@ export const BrowserTab: React.FC<BrowserTabProps> = React.memo(
751752
],
752753
);
753754

755+
/**
756+
* Handles target="_blank" links and window.open() calls.
757+
* On Android, without this handler, such links open in the system browser (Chrome)
758+
* because the native WebChromeClient creates a bare WebView with no fallback.
759+
* On iOS, the native fallback loads the URL in the same WebView, but providing
760+
* this explicit handler ensures consistent behavior across both platforms.
761+
*/
762+
const handleOpenWindow = useCallback((event: WebViewOpenWindowEvent) => {
763+
const { targetUrl } = event.nativeEvent;
764+
if (targetUrl && webviewRef.current) {
765+
webviewRef.current.injectJavaScript(
766+
`window.location.href = '${sanitizeUrlInput(targetUrl)}'; true;`,
767+
);
768+
}
769+
}, []);
770+
754771
/**
755772
* Sets loading bar progress
756773
*/
@@ -1493,6 +1510,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = React.memo(
14931510
onShouldStartLoadWithRequest
14941511
}
14951512
allowsInlineMediaPlayback
1513+
onOpenWindow={handleOpenWindow}
14961514
{...webViewTestProps}
14971515
testID={BrowserViewSelectorsIDs.BROWSER_WEBVIEW_ID}
14981516
applicationNameForUserAgent={'WebView MetaMaskMobile'}

app/components/Views/BrowserTab/__snapshots__/index.test.tsx.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,7 @@ exports[`BrowserTab render Browser 1`] = `
568568
onLoadStart={[Function]}
569569
onMessage={[Function]}
570570
onNavigationStateChange={[Function]}
571+
onOpenWindow={[Function]}
571572
onShouldStartLoadWithRequest={[Function]}
572573
originWhitelist={
573574
[

app/components/Views/BrowserTab/index.test.tsx

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,29 @@ import { backgroundState } from '../../../util/test/initial-root-state';
66
import { MOCK_ACCOUNTS_CONTROLLER_STATE } from '../../../util/test/accountsControllerTestUtils';
77
import BrowserTab from './BrowserTab';
88

9+
const mockInjectJavaScript = jest.fn();
10+
11+
jest.mock('@metamask/react-native-webview', () => {
12+
const { View } = jest.requireActual('react-native');
13+
const ActualReact = jest.requireActual('react');
14+
15+
const MockWebView = ActualReact.forwardRef(
16+
(props: Record<string, unknown>, ref: React.Ref<unknown>) => {
17+
ActualReact.useImperativeHandle(ref, () => ({
18+
injectJavaScript: mockInjectJavaScript,
19+
}));
20+
return <View {...props} />;
21+
},
22+
);
23+
MockWebView.displayName = 'WebView';
24+
25+
return {
26+
__esModule: true,
27+
WebView: MockWebView,
28+
default: MockWebView,
29+
};
30+
});
31+
932
const mockNavigation = {
1033
goBack: jest.fn(),
1134
goForward: jest.fn(),
@@ -90,6 +113,7 @@ const mockProps = {
90113
describe('BrowserTab', () => {
91114
beforeEach(() => {
92115
jest.clearAllMocks();
116+
mockInjectJavaScript.mockClear();
93117
});
94118

95119
it('render Browser', async () => {
@@ -215,4 +239,83 @@ describe('BrowserTab', () => {
215239
).toBe(false);
216240
});
217241
});
242+
243+
describe('WebView onOpenWindow', () => {
244+
it('passes onOpenWindow handler to WebView', async () => {
245+
renderWithProvider(<BrowserTab {...mockProps} />, {
246+
state: mockInitialState,
247+
});
248+
249+
await waitFor(() =>
250+
expect(screen.getByTestId('browser-webview')).toBeVisible(),
251+
);
252+
253+
const webView = screen.getByTestId('browser-webview');
254+
255+
expect(typeof webView.props.onOpenWindow).toBe('function');
256+
});
257+
258+
it('calls injectJavaScript with sanitized target URL when onOpenWindow fires', async () => {
259+
renderWithProvider(<BrowserTab {...mockProps} />, {
260+
state: mockInitialState,
261+
});
262+
263+
await waitFor(() =>
264+
expect(screen.getByTestId('browser-webview')).toBeVisible(),
265+
);
266+
267+
const webView = screen.getByTestId('browser-webview');
268+
const { onOpenWindow } = webView.props;
269+
270+
onOpenWindow({
271+
nativeEvent: { targetUrl: 'https://stake.lido.fi' },
272+
});
273+
274+
expect(mockInjectJavaScript).toHaveBeenCalledTimes(1);
275+
expect(mockInjectJavaScript).toHaveBeenCalledWith(
276+
"window.location.href = 'https://stake.lido.fi'; true;",
277+
);
278+
});
279+
280+
it('sanitizes single quotes in the target URL before injecting', async () => {
281+
renderWithProvider(<BrowserTab {...mockProps} />, {
282+
state: mockInitialState,
283+
});
284+
285+
await waitFor(() =>
286+
expect(screen.getByTestId('browser-webview')).toBeVisible(),
287+
);
288+
289+
const webView = screen.getByTestId('browser-webview');
290+
const { onOpenWindow } = webView.props;
291+
292+
onOpenWindow({
293+
nativeEvent: { targetUrl: "https://example.com/path?q='test'" },
294+
});
295+
296+
expect(mockInjectJavaScript).toHaveBeenCalledTimes(1);
297+
expect(mockInjectJavaScript).toHaveBeenCalledWith(
298+
"window.location.href = 'https://example.com/path?q=%27test%27'; true;",
299+
);
300+
});
301+
302+
it('does not call injectJavaScript when targetUrl is empty', async () => {
303+
renderWithProvider(<BrowserTab {...mockProps} />, {
304+
state: mockInitialState,
305+
});
306+
307+
await waitFor(() =>
308+
expect(screen.getByTestId('browser-webview')).toBeVisible(),
309+
);
310+
311+
const webView = screen.getByTestId('browser-webview');
312+
const { onOpenWindow } = webView.props;
313+
314+
onOpenWindow({
315+
nativeEvent: { targetUrl: '' },
316+
});
317+
318+
expect(mockInjectJavaScript).not.toHaveBeenCalled();
319+
});
320+
});
218321
});

0 commit comments

Comments
 (0)