From 355933060eb59f7ed3fb3eed23933d562cc5b6a6 Mon Sep 17 00:00:00 2001 From: Adam Iskounen <44589599+iskounen@users.noreply.github.com> Date: Tue, 10 Oct 2023 12:50:50 -0400 Subject: [PATCH] feat: prevent backward navigation when clicking a link in a webview (EMI-1395) (#9389) * prevented backward navigation when clicking a link in a webview * simplified stopLoading behavior * wrote tests for new behavior * undid the simplification changes * mock debounce for the test * shortened a conditional * updated confusing test case message --------- Co-authored-by: Sultan --- src/app/Components/ArtsyWebView.tests.tsx | 93 ++++++++++++++++++++++- src/app/Components/ArtsyWebView.tsx | 13 ++-- 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/src/app/Components/ArtsyWebView.tests.tsx b/src/app/Components/ArtsyWebView.tests.tsx index 788be111e57..aacb1763c09 100644 --- a/src/app/Components/ArtsyWebView.tests.tsx +++ b/src/app/Components/ArtsyWebView.tests.tsx @@ -5,6 +5,7 @@ import { goBack, navigate } from "app/system/navigation/navigate" import { appJson } from "app/utils/jsonFiles" import { renderWithWrappers } from "app/utils/tests/renderWithWrappers" import mockFetch from "jest-fetch-mock" +import { debounce } from "lodash" import { stringify } from "query-string" import Share from "react-native-share" import WebView, { WebViewProps } from "react-native-webview" @@ -24,6 +25,36 @@ jest.mock("app/utils/useWebViewEvent", () => ({ }), })) +// mock implementation of an inner WebView's goBack method that can be observed +const mockGoBack = jest.fn() + +// mocked ref to the inner WebView of an ArtsyWebView whose properties can be observed +const mockRef = { + current: { + goBack: mockGoBack, + stopLoading: jest.fn(), + }, +} + +// mock implementation of the react-native-webview that allows us to observe its properties +jest.mock("react-native-webview", () => { + const React = require("react") + const { View } = require("react-native") + + return { + __esModule: true, + default: React.forwardRef((props: any, ref: any) => { + React.useImperativeHandle(ref, () => mockRef.current) + return + }), + } +}) + +jest.mock("lodash", () => ({ + ...jest.requireActual("lodash"), + debounce: jest.fn(), +})) + const mockOnNavigationStateChange: WebViewNavigation = { navigationType: "click", url: "https://gooooogle.com", @@ -42,7 +73,10 @@ describe("ArtsyWebView", () => { }) describe("ArtsyWebViewPage", () => { - beforeEach(() => jest.clearAllMocks()) + beforeEach(() => { + jest.clearAllMocks() + ;(debounce as jest.Mock).mockImplementation((func) => func) + }) const render = (props: Partial> = {}) => renderWithWrappers() @@ -206,6 +240,63 @@ describe("ArtsyWebViewPage", () => { }) expect(navigate).toHaveBeenCalledWith("https://google.com") }) + + describe("the inner WebView's goBack method", () => { + it("is called when the URL matches a route that is not loaded in a web view", () => { + const tree = render() + + webViewProps(tree).onNavigationStateChange?.({ + ...mockOnNavigationStateChange, + url: "https://staging.artsy.net/artwork/foo", + }) + + expect(mockGoBack).toHaveBeenCalled() + }) + + it("is not called when the URL does not match any route", () => { + const tree = render() + + webViewProps(tree).onNavigationStateChange?.({ + ...mockOnNavigationStateChange, + url: "https://support.artsy.net/", + }) + + expect(mockGoBack).not.toHaveBeenCalled() + }) + + it("is not called when the URL matches a ModalWebView route", () => { + const tree = render() + + webViewProps(tree).onNavigationStateChange?.({ + ...mockOnNavigationStateChange, + url: "https://staging.artsy.net/orders/foo", + }) + + expect(mockGoBack).not.toHaveBeenCalled() + }) + + it("is not called when the URL matches a ReactWebView route", () => { + const tree = render() + + webViewProps(tree).onNavigationStateChange?.({ + ...mockOnNavigationStateChange, + url: "https://staging.artsy.net/meet-the-specialists", + }) + + expect(mockGoBack).not.toHaveBeenCalled() + }) + + it("is not called when the URL matches a VanityURLEntity route", () => { + const tree = render() + + webViewProps(tree).onNavigationStateChange?.({ + ...mockOnNavigationStateChange, + url: "https://staging.artsy.net/foo", + }) + + expect(mockGoBack).not.toHaveBeenCalled() + }) + }) }) }) diff --git a/src/app/Components/ArtsyWebView.tsx b/src/app/Components/ArtsyWebView.tsx index 2624d8db696..28574e8d99e 100644 --- a/src/app/Components/ArtsyWebView.tsx +++ b/src/app/Components/ArtsyWebView.tsx @@ -189,9 +189,12 @@ export const ArtsyWebView = forwardRef< const uri = url.startsWith("/") ? webURL + url : url // Debounce calls just in case multiple stopLoading calls are made in a row - const stopLoading = debounce(() => { + const stopLoading = debounce((needToGoBack = true) => { innerRef.current?.stopLoading() - innerRef.current?.goBack() + + if (needToGoBack) { + innerRef.current?.goBack() + } }, 500) const onNavigationStateChange = (evt: WebViewNavigation) => { @@ -215,13 +218,13 @@ export const ArtsyWebView = forwardRef< // to a different vanityURL that we can handle inapp, such as Fair & Partner. if ( result.type === "match" && - (["ReactWebView", "ModalWebView"].includes(result.module) || - result.module === "VanityURLEntity") + ["ReactWebView", "ModalWebView", "VanityURLEntity"].includes(result.module) ) { innerRef.current!.shareTitleUrl = targetURL return } else { - stopLoading() + const needToGoBack = result.type !== "external_url" + stopLoading(needToGoBack) } // In case of a webview presented modally, if the targetURL is a tab View,