diff --git a/src/components/FloatingToolbar/index.jsx b/src/components/FloatingToolbar/index.jsx index e44665ec1..2b149c1de 100644 --- a/src/components/FloatingToolbar/index.jsx +++ b/src/components/FloatingToolbar/index.jsx @@ -1,4 +1,5 @@ import { cloneElement, useCallback, useEffect, useState } from 'react' +import { unmountComponentAtNode } from 'react-dom' import ConversationCard from '../ConversationCard' import PropTypes from 'prop-types' import { config as toolsConfig } from '../../content-script/selection-tools' @@ -70,6 +71,7 @@ function FloatingToolbar(props) { } const onClose = useCallback(() => { + unmountComponentAtNode(props.container) props.container.remove() }, []) diff --git a/src/content-script/index.jsx b/src/content-script/index.jsx index 05f65c53d..c49164a8b 100644 --- a/src/content-script/index.jsx +++ b/src/content-script/index.jsx @@ -225,10 +225,13 @@ async function getInput(inputQuery) { } let toolbarContainer +let toolbarCreationVersion = 0 const deleteToolbar = () => { + toolbarCreationVersion += 1 try { if (toolbarContainer && toolbarContainer.className === 'chatgptbox-toolbar-container') { console.debug('[content] Deleting toolbar:', toolbarContainer) + unmountComponentAtNode(toolbarContainer) toolbarContainer.remove() toolbarContainer = null } @@ -237,7 +240,7 @@ const deleteToolbar = () => { } } -const createSelectionTools = async (toolbarContainerElement, selection) => { +const createSelectionTools = async (toolbarContainerElement, selection, creationVersion) => { console.debug( '[content] createSelectionTools called with selection:', selection, @@ -247,6 +250,14 @@ const createSelectionTools = async (toolbarContainerElement, selection) => { try { toolbarContainerElement.className = 'chatgptbox-toolbar-container' const userConfig = await getUserConfig() + if ( + creationVersion !== toolbarCreationVersion || + toolbarContainerElement !== toolbarContainer || + !toolbarContainerElement.isConnected + ) { + console.debug('[content] Selection tools creation was superseded, skipping render.') + return + } render( { try { + if (creationVersion !== toolbarCreationVersion) return const selection = window .getSelection() ?.toString() @@ -302,6 +315,7 @@ async function prepareForSelectionTools() { let position const config = await getUserConfig() + if (creationVersion !== toolbarCreationVersion) return if (!config.selectionToolsNextToInputBox) { position = { x: e.pageX + 20, y: e.pageY + 20 } } else { @@ -325,8 +339,9 @@ async function prepareForSelectionTools() { } } console.debug('[content] Toolbar position:', position) - toolbarContainer = createElementAtPosition(position.x, position.y) - await createSelectionTools(toolbarContainer, selection) + const container = createElementAtPosition(position.x, position.y) + toolbarContainer = container + await createSelectionTools(container, selection, creationVersion) } else { console.debug('[content] No text selected on mouseup.') } @@ -346,7 +361,11 @@ async function prepareForSelectionTools() { return } console.debug('[content] Mousedown outside toolbar, removing existing toolbars.') - document.querySelectorAll('.chatgptbox-toolbar-container').forEach((el) => el.remove()) + toolbarCreationVersion += 1 + document.querySelectorAll('.chatgptbox-toolbar-container').forEach((el) => { + unmountComponentAtNode(el) + el.remove() + }) toolbarContainer = null } catch (error) { console.error('[content] Error in mousedown listener for selection tools:', error) @@ -402,8 +421,10 @@ async function prepareForSelectionToolsTouch() { } deleteToolbar() + const creationVersion = toolbarCreationVersion setTimeout(async () => { try { + if (creationVersion !== toolbarCreationVersion) return const selection = window .getSelection() ?.toString() @@ -412,8 +433,9 @@ async function prepareForSelectionToolsTouch() { if (selection) { console.debug('[content] Text selected via touch:', selection) const touch = e.changedTouches[0] - toolbarContainer = createElementAtPosition(touch.pageX + 20, touch.pageY + 20) - await createSelectionTools(toolbarContainer, selection) + const container = createElementAtPosition(touch.pageX + 20, touch.pageY + 20) + toolbarContainer = container + await createSelectionTools(container, selection, creationVersion) } else { console.debug('[content] No text selected on touchend.') } @@ -436,7 +458,11 @@ async function prepareForSelectionToolsTouch() { return } console.debug('[content] Touchstart outside toolbar, removing existing toolbars.') - document.querySelectorAll('.chatgptbox-toolbar-container').forEach((el) => el.remove()) + toolbarCreationVersion += 1 + document.querySelectorAll('.chatgptbox-toolbar-container').forEach((el) => { + unmountComponentAtNode(el) + el.remove() + }) toolbarContainer = null } catch (error) { console.error('[content] Error in touchstart listener for touch selection tools:', error) diff --git a/tests/setup/content-script-selection-toolbar-loader-hooks.mjs b/tests/setup/content-script-selection-toolbar-loader-hooks.mjs new file mode 100644 index 000000000..d72a7ed13 --- /dev/null +++ b/tests/setup/content-script-selection-toolbar-loader-hooks.mjs @@ -0,0 +1,169 @@ +import { readFile } from 'node:fs/promises' +import { fileURLToPath } from 'node:url' + +const contentScriptStubs = new Map([ + ['./styles.scss', 'test:styles'], + ['../components/DecisionCard', 'test:decision-card'], + ['./site-adapters', 'test:site-adapters'], + ['./selection-tools', 'test:selection-tools'], + ['./menu-tools', 'test:menu-tools'], + ['../config/index.mjs', 'test:config'], + ['../utils', 'test:utils'], + ['../components/FloatingToolbar', 'test:floating-toolbar'], + ['webextension-polyfill', 'test:browser'], + ['../config/language.mjs', 'test:language'], + ['../_locales/i18n-react', 'test:i18n-react'], + ['i18next', 'test:i18next'], + ['../services/init-session.mjs', 'test:init-session'], + ['../services/wrappers.mjs', 'test:wrappers'], + ['../services/apis/chatgpt-web.mjs', 'test:chatgpt-api'], + ['../components/WebJumpBackNotification', 'test:jump-back'], + ['./port-error.mjs', 'test:port-error'], +]) + +const floatingToolbarStubs = new Map([ + ['../ConversationCard', 'test:floating-conversation-card'], + ['../../content-script/selection-tools', 'test:floating-selection-tools'], + ['../../utils', 'test:floating-utils'], + ['react-draggable', 'test:floating-draggable'], + ['../../hooks/use-clamp-window-size', 'test:floating-window-size'], + ['react-i18next', 'test:floating-i18n'], + ['../../hooks/use-config.mjs', 'test:floating-config'], +]) + +const sources = { + 'test:styles': '', + 'test:decision-card': 'export default function DecisionCard() { return null }', + 'test:site-adapters': 'export const config = {}', + 'test:selection-tools': 'export const config = {}', + 'test:menu-tools': 'export const config = {}', + 'test:config': ` + export const chatgptWebModelKeys = [] + export const getPreferredLanguageKey = async () => 'en' + export const getUserConfig = () => globalThis.__SELECTION_TOOLBAR_TEST__.getUserConfig() + export const isUsingChatgptWebModel = () => false + export const setAccessToken = async () => {} + export const setUserConfig = async () => {} + `, + 'test:utils': ` + export const createElementAtPosition = () => { + const element = document.createElement('div') + document.documentElement.append(element) + globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.push(element) + return element + } + export const cropText = async (text) => text + export const endsWithQuestionMark = () => false + export const getApiModesStringArrayFromConfig = () => [] + export const getClientPosition = () => ({ x: 0, y: 0 }) + export const getPossibleElementByQuerySelector = () => null + `, + 'test:floating-toolbar': ` + export default function FloatingToolbar() { + globalThis.__SELECTION_TOOLBAR_TEST__.renderCount += 1 + return null + } + `, + 'test:browser': ` + const event = { addListener() {}, removeListener() {} } + export default { + runtime: { onMessage: event, sendMessage: async () => {} }, + storage: { onChanged: event }, + } + `, + 'test:language': `export const getPreferredLanguage = async () => 'English'`, + 'test:i18n-react': '', + 'test:i18next': 'export const changeLanguage = async () => {}', + 'test:init-session': 'export const initSession = () => ({})', + 'test:wrappers': ` + export const getChatGptAccessToken = async () => null + export const registerPortListener = () => {} + `, + 'test:chatgpt-api': 'export const generateAnswersWithChatgptWebApi = async () => {}', + 'test:jump-back': 'export default function WebJumpBackNotification() { return null }', + 'test:port-error': ` + export const getPortErrorMessage = (error) => String(error) + export const shouldDelegatePortError = () => false + `, + 'test:floating-conversation-card': ` + import { useLayoutEffect } from 'preact/hooks' + export default function ConversationCard(props) { + const state = globalThis.__FLOATING_TOOLBAR_TEST__ + state.onClose = props.onClose + useLayoutEffect(() => () => { + state.cleanupCount += 1 + state.cleanupSawConnectedContainer = state.container.isConnected + }, []) + return null + } + `, + 'test:floating-selection-tools': 'export const config = {}', + 'test:floating-utils': ` + export const getClientPosition = () => ({ x: 0, y: 0 }) + export const isMobile = () => false + export const setElementPositionInViewport = (_container, x, y) => ({ x, y }) + `, + 'test:floating-draggable': ` + export default function Draggable(props) { + return props.children + } + `, + 'test:floating-window-size': 'export const useClampWindowSize = () => [1000, 1000]', + 'test:floating-i18n': 'export const useTranslation = () => ({ t: (value) => value })', + 'test:floating-config': ` + import { useLayoutEffect } from 'preact/hooks' + const config = { + alwaysPinWindow: false, + themeMode: 'light', + activeSelectionTools: [], + customSelectionTools: [], + } + export const useConfig = (onLoad) => { + useLayoutEffect(() => { + onLoad() + }, []) + return config + } + `, +} + +export async function resolve(specifier, context, nextResolve) { + if (context.parentURL?.startsWith('test:') && specifier === 'preact/hooks') { + return nextResolve(specifier, { ...context, parentURL: import.meta.url }) + } + + if (context.parentURL?.endsWith('/src/content-script/index.jsx')) { + const stubUrl = contentScriptStubs.get(specifier) + if (stubUrl) return { url: stubUrl, shortCircuit: true } + } + + if (context.parentURL?.endsWith('/src/components/FloatingToolbar/index.jsx')) { + const stubUrl = floatingToolbarStubs.get(specifier) + if (stubUrl) return { url: stubUrl, shortCircuit: true } + } + + return nextResolve(specifier, context) +} + +export async function load(url, context, nextLoad) { + if (url.startsWith('test:')) { + return { + shortCircuit: true, + format: 'module', + source: sources[url], + } + } + + if (url.startsWith('file://') && url.endsWith('.jsx') && !url.includes('node_modules')) { + const source = await readFile(fileURLToPath(url), 'utf8') + const esbuild = await import('esbuild') + const result = await esbuild.transform(source, { + loader: 'jsx', + jsx: 'automatic', + jsxImportSource: 'preact', + }) + return { shortCircuit: true, format: 'module', source: result.code } + } + + return nextLoad(url, context) +} diff --git a/tests/unit/content-script/selection-toolbar-lifecycle.test.mjs b/tests/unit/content-script/selection-toolbar-lifecycle.test.mjs new file mode 100644 index 000000000..d022adb35 --- /dev/null +++ b/tests/unit/content-script/selection-toolbar-lifecycle.test.mjs @@ -0,0 +1,302 @@ +import assert from 'node:assert/strict' +import { register } from 'node:module' +import { cwd } from 'node:process' +import { after, before, test } from 'node:test' +import { pathToFileURL } from 'node:url' +import { JSDOM } from 'jsdom' +import { h, render } from 'preact' +import { useLayoutEffect } from 'preact/hooks' + +register( + './tests/setup/content-script-selection-toolbar-loader-hooks.mjs', + pathToFileURL(cwd() + '/').href, +) + +const baseConfig = { + alwaysFloatingSidebar: false, + inputQuery: '', + prependQuery: '', + appendQuery: '', + selectionToolsNextToInputBox: false, + useSiteRegexOnly: true, + siteRegex: '', + siteAdapters: [], + activeSiteAdapters: [], + activeApiModes: [], + customApiModes: [], + modelName: 'test-model', + apiMode: 'test-mode', + customModelName: '', +} + +const nextTask = () => new Promise((resolve) => setTimeout(resolve, 0)) + +const waitFor = async (predicate, message) => { + for (let attempt = 0; attempt < 50; ++attempt) { + if (predicate()) return + await nextTask() + } + assert.fail(message) +} + +const deferred = () => { + let resolve + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise + }) + return { promise, resolve } +} + +const dispatchTouchEvent = (type, pageX = 20, pageY = 20) => { + const event = new Event(type, { bubbles: true }) + Object.defineProperty(event, 'changedTouches', { + value: [{ pageX, pageY }], + }) + document.body.dispatchEvent(event) +} + +const mountCleanupProbe = (container) => { + const cleanup = { + count: 0, + sawConnectedContainer: false, + } + + function CleanupProbe() { + useLayoutEffect( + () => () => { + cleanup.count += 1 + cleanup.sawConnectedContainer = container.isConnected + }, + [], + ) + return null + } + + render(h(CleanupProbe), container) + return cleanup +} + +let dom +let FloatingToolbar +let selectionText = 'selected text' +const globalDescriptors = new Map() +const globalNames = ['window', 'document', 'location', 'Node', 'Event', 'MouseEvent', 'HTMLElement'] + +before(async () => { + dom = new JSDOM('', { url: 'https://example.com/' }) + + for (const name of globalNames) { + globalDescriptors.set(name, Object.getOwnPropertyDescriptor(globalThis, name)) + Object.defineProperty(globalThis, name, { + configurable: true, + value: dom.window[name], + }) + } + + globalThis.__SELECTION_TOOLBAR_TEST__ = { + createdContainers: [], + renderCount: 0, + getUserConfig: async () => baseConfig, + } + globalThis.__FLOATING_TOOLBAR_TEST__ = { + onClose: null, + cleanupCount: 0, + cleanupSawConnectedContainer: false, + container: null, + } + + Object.defineProperty(window, 'getSelection', { + configurable: true, + value: () => ({ + rangeCount: 0, + toString: () => selectionText, + }), + }) + + await import('../../../src/content-script/index.jsx') + ;({ default: FloatingToolbar } = await import( + '../../../src/components/FloatingToolbar/index.jsx' + )) + await nextTask() + await nextTask() +}) + +after(() => { + dom.window.close() + delete globalThis.__SELECTION_TOOLBAR_TEST__ + delete globalThis.__FLOATING_TOOLBAR_TEST__ + + for (const [name, descriptor] of globalDescriptors) { + if (descriptor) Object.defineProperty(globalThis, name, descriptor) + else delete globalThis[name] + } +}) + +test('outside interaction unmounts a toolbar before removing its container', () => { + const container = document.createElement('div') + container.className = 'chatgptbox-toolbar-container' + document.documentElement.append(container) + const cleanup = mountCleanupProbe(container) + + document.body.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })) + + assert.equal(cleanup.count, 1) + assert.equal(cleanup.sawConnectedContainer, true) + assert.equal(container.isConnected, false) +}) + +test('tracked toolbar deletion unmounts before removing its container', async () => { + const createdBefore = globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.length + const renderedBefore = globalThis.__SELECTION_TOOLBAR_TEST__.renderCount + + selectionText = 'selected text' + document.body.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })) + await waitFor( + () => + globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.length === createdBefore + 1 && + globalThis.__SELECTION_TOOLBAR_TEST__.renderCount === renderedBefore + 1, + 'tracked selection toolbar was not rendered', + ) + + const container = globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.at(-1) + const cleanup = mountCleanupProbe(container) + + selectionText = '' + document.body.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })) + + assert.equal(cleanup.count, 1) + assert.equal(cleanup.sawConnectedContainer, true) + assert.equal(container.isConnected, false) + + await nextTask() + selectionText = 'selected text' +}) + +test('toolbar close unmounts its component tree before removing the container', async () => { + const state = globalThis.__FLOATING_TOOLBAR_TEST__ + const container = document.createElement('div') + container.className = 'chatgptbox-toolbar-container' + document.documentElement.append(container) + + state.onClose = null + state.cleanupCount = 0 + state.cleanupSawConnectedContainer = false + state.container = container + + render( + h(FloatingToolbar, { + session: {}, + selection: 'selected text', + container, + triggered: true, + closeable: true, + dockable: false, + prompt: 'prompt', + }), + container, + ) + + await waitFor( + () => typeof state.onClose === 'function', + 'toolbar close callback was not rendered', + ) + state.onClose() + + assert.equal(state.cleanupCount, 1) + assert.equal(state.cleanupSawConnectedContainer, true) + assert.equal(container.isConnected, false) +}) + +test('cancelling while the first mouse config read is pending prevents container creation', async () => { + const pendingConfig = deferred() + const createdBefore = globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.length + const renderedBefore = globalThis.__SELECTION_TOOLBAR_TEST__.renderCount + + globalThis.__SELECTION_TOOLBAR_TEST__.getUserConfig = () => pendingConfig.promise + + document.body.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })) + await nextTask() + await nextTask() + + document.body.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })) + pendingConfig.resolve(baseConfig) + await nextTask() + await nextTask() + + assert.equal(globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.length, createdBefore) + assert.equal(globalThis.__SELECTION_TOOLBAR_TEST__.renderCount, renderedBefore) + globalThis.__SELECTION_TOOLBAR_TEST__.getUserConfig = async () => baseConfig +}) + +test('removing a mouse container while its render config is pending prevents render', async () => { + const pendingRenderConfig = deferred() + const createdBefore = globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.length + const renderedBefore = globalThis.__SELECTION_TOOLBAR_TEST__.renderCount + let configRead = 0 + + globalThis.__SELECTION_TOOLBAR_TEST__.getUserConfig = () => { + configRead += 1 + if (configRead === 1) return Promise.resolve(baseConfig) + if (configRead === 2) return pendingRenderConfig.promise + return Promise.resolve(baseConfig) + } + + document.body.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })) + await waitFor( + () => globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.length === createdBefore + 1, + 'selection toolbar container was not created', + ) + + const container = globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.at(-1) + assert.equal(container.isConnected, true) + + document.body.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })) + assert.equal(container.isConnected, false) + + pendingRenderConfig.resolve(baseConfig) + await nextTask() + await nextTask() + + assert.equal(globalThis.__SELECTION_TOOLBAR_TEST__.renderCount, renderedBefore) + globalThis.__SELECTION_TOOLBAR_TEST__.getUserConfig = async () => baseConfig +}) + +test('touchstart unmounts a toolbar before removing its container', () => { + const container = document.createElement('div') + container.className = 'chatgptbox-toolbar-container' + document.documentElement.append(container) + const cleanup = mountCleanupProbe(container) + + dispatchTouchEvent('touchstart') + + assert.equal(cleanup.count, 1) + assert.equal(cleanup.sawConnectedContainer, true) + assert.equal(container.isConnected, false) +}) + +test('touchstart cancels touch toolbar rendering while config is pending', async () => { + const pendingConfig = deferred() + const createdBefore = globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.length + const renderedBefore = globalThis.__SELECTION_TOOLBAR_TEST__.renderCount + + globalThis.__SELECTION_TOOLBAR_TEST__.getUserConfig = () => pendingConfig.promise + + dispatchTouchEvent('touchend') + await waitFor( + () => globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.length === createdBefore + 1, + 'touch selection toolbar container was not created', + ) + + const container = globalThis.__SELECTION_TOOLBAR_TEST__.createdContainers.at(-1) + assert.equal(container.isConnected, true) + + dispatchTouchEvent('touchstart') + assert.equal(container.isConnected, false) + + pendingConfig.resolve(baseConfig) + await nextTask() + await nextTask() + + assert.equal(globalThis.__SELECTION_TOOLBAR_TEST__.renderCount, renderedBefore) + globalThis.__SELECTION_TOOLBAR_TEST__.getUserConfig = async () => baseConfig +})