From 7d47cc52b696080ab3810ab5a64e266a4e782cfb Mon Sep 17 00:00:00 2001 From: DarkSky <25152247+darkskygit@users.noreply.github.com> Date: Tue, 27 Jan 2026 00:54:21 +0800 Subject: [PATCH] fix: firefox input (#14315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix #14296 fix #14289 #### PR Dependency Tree * **PR #14315** 👈 This tree was auto-generated by [Charcoal](https://github.com/danerwilliams/charcoal) ## Summary by CodeRabbit * **Bug Fixes** * Improved inline editor stability for selection edge cases and beforeinput handling, with better recovery and native-input protection. * Fixed potential crashes when deleting with selections outside the editor bounds, including Firefox-specific scenarios. * **Tests** * Added unit tests covering beforeinput behavior and added Firefox end-to-end regression tests. * **Chores** * Reduced CI test parallelism to streamline pipeline. ✏️ Tip: You can customize this high-level summary in your review settings. --- .github/workflows/build-test.yml | 33 ++-- .../inline/event-beforeinput.unit.spec.ts | 144 ++++++++++++++++++ .../std/src/inline/services/event.ts | 134 ++++++++++++---- .../inline/firefox-beforeinput.spec.ts | 87 +++++++++++ 4 files changed, 344 insertions(+), 54 deletions(-) create mode 100644 blocksuite/framework/std/src/__tests__/inline/event-beforeinput.unit.spec.ts create mode 100644 tests/blocksuite/e2e/cross-platform/inline/firefox-beforeinput.spec.ts diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index cae2d6cd62..3a0b85fc12 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -182,7 +182,7 @@ jobs: strategy: fail-fast: false matrix: - shard: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + shard: [1, 2] steps: - uses: actions/checkout@v4 - name: Setup Node.js @@ -213,7 +213,7 @@ jobs: strategy: fail-fast: false matrix: - shard: [1, 2] + shard: [1] browser: ['chromium', 'firefox', 'webkit'] steps: - uses: actions/checkout@v4 @@ -251,7 +251,7 @@ jobs: strategy: fail-fast: false matrix: - shard: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] + shard: [1, 2, 3, 4, 5] steps: - uses: actions/checkout@v4 - name: Setup Node.js @@ -282,7 +282,7 @@ jobs: strategy: fail-fast: false matrix: - shard: [1, 2, 3, 4, 5] + shard: [1, 2] steps: - uses: actions/checkout@v4 - name: Setup Node.js @@ -313,7 +313,7 @@ jobs: strategy: fail-fast: false matrix: - shard: [1, 2, 3, 4, 5] + shard: [1, 2, 3] steps: - uses: actions/checkout@v4 - name: Setup Node.js @@ -507,8 +507,8 @@ jobs: strategy: fail-fast: false matrix: - node_index: [0, 1, 2, 3, 4, 5, 6, 7] - total_nodes: [8] + node_index: [0, 1, 2, 3] + total_nodes: [4] env: NODE_ENV: test DATABASE_URL: postgresql://affine:affine@localhost:5432/affine @@ -924,8 +924,8 @@ jobs: strategy: fail-fast: false matrix: - shardIndex: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10] - shardTotal: [10] + shardIndex: [1, 2, 3, 4, 5] + shardTotal: [5] needs: - build-server-native services: @@ -1031,21 +1031,6 @@ jobs: - name: 'Cloud E2E Test 5/10' shard: 5 script: yarn affine @affine-test/affine-cloud e2e --forbid-only --shard=5/10 - - name: 'Cloud E2E Test 6/10' - shard: 6 - script: yarn affine @affine-test/affine-cloud e2e --forbid-only --shard=6/10 - - name: 'Cloud E2E Test 7/10' - shard: 7 - script: yarn affine @affine-test/affine-cloud e2e --forbid-only --shard=7/10 - - name: 'Cloud E2E Test 8/10' - shard: 8 - script: yarn affine @affine-test/affine-cloud e2e --forbid-only --shard=8/10 - - name: 'Cloud E2E Test 9/10' - shard: 9 - script: yarn affine @affine-test/affine-cloud e2e --forbid-only --shard=9/10 - - name: 'Cloud E2E Test 10/10' - shard: 10 - script: yarn affine @affine-test/affine-cloud e2e --forbid-only --shard=10/10 - name: 'Cloud Desktop E2E Test' shard: desktop script: | diff --git a/blocksuite/framework/std/src/__tests__/inline/event-beforeinput.unit.spec.ts b/blocksuite/framework/std/src/__tests__/inline/event-beforeinput.unit.spec.ts new file mode 100644 index 0000000000..8133a9df44 --- /dev/null +++ b/blocksuite/framework/std/src/__tests__/inline/event-beforeinput.unit.spec.ts @@ -0,0 +1,144 @@ +import { expect, test, vi } from 'vitest'; +import * as Y from 'yjs'; + +import { effects } from '../../effects.js'; +import { InlineEditor } from '../../inline/index.js'; + +effects(); + +async function setupInlineEditor(text: string) { + const yDoc = new Y.Doc(); + const yText = yDoc.getText('text'); + yText.insert(0, text); + + const editor = new InlineEditor(yText); + const root = document.createElement('div'); + const outside = document.createElement('div'); + outside.textContent = 'outside'; + + document.body.append(root, outside); + editor.mount(root); + await editor.waitForUpdate(); + + return { editor, root, outside }; +} + +function setNativeSelection(range: Range) { + const selection = document.getSelection(); + if (!selection) { + throw new Error('Selection is not available'); + } + selection.removeAllRanges(); + selection.addRange(range); +} + +function clearNativeSelection() { + const selection = document.getSelection(); + selection?.removeAllRanges(); +} + +async function teardownInlineEditor( + ctx: Awaited> +) { + clearNativeSelection(); + ctx.editor.unmount(); + ctx.root.remove(); + ctx.outside.remove(); +} + +test('beforeinput prevents native edits for selection partially outside inline root', async () => { + const ctx = await setupInlineEditor('hello'); + try { + const range = ctx.editor.toDomRange({ index: 1, length: 0 }); + expect(range).not.toBeNull(); + range!.setEnd(ctx.outside, 0); + setNativeSelection(range!); + + const preventDefault = vi.fn(); + const event = { + inputType: 'deleteContentForward', + data: null, + dataTransfer: null, + preventDefault, + stopPropagation: vi.fn(), + getTargetRanges: () => [], + } as unknown as InputEvent; + + await (ctx.editor.eventService as any)._onBeforeInput(event); + + expect(preventDefault).toHaveBeenCalledOnce(); + expect(ctx.editor.yTextString).toBe('h'); + } finally { + await teardownInlineEditor(ctx); + } +}); + +test('beforeinput does not intercept when selection spans another inline root', async () => { + const ctx1 = await setupInlineEditor('abc'); + const ctx2 = await setupInlineEditor('xyz'); + try { + const startRange = ctx1.editor.toDomRange({ index: 1, length: 0 }); + const endRange = ctx2.editor.toDomRange({ index: 1, length: 0 }); + expect(startRange).not.toBeNull(); + expect(endRange).not.toBeNull(); + + const selectionRange = document.createRange(); + selectionRange.setStart( + startRange!.startContainer, + startRange!.startOffset + ); + selectionRange.setEnd(endRange!.endContainer, endRange!.endOffset); + setNativeSelection(selectionRange); + + const preventDefault = vi.fn(); + const event = { + inputType: 'deleteContentForward', + data: null, + dataTransfer: null, + preventDefault, + stopPropagation: vi.fn(), + getTargetRanges: () => [], + } as unknown as InputEvent; + + await (ctx1.editor.eventService as any)._onBeforeInput(event); + + expect(preventDefault).not.toHaveBeenCalled(); + expect(ctx1.editor.yTextString).toBe('abc'); + } finally { + await teardownInlineEditor(ctx1); + await teardownInlineEditor(ctx2); + } +}); + +test('beforeinput ignores un-resolvable target range and still applies input', async () => { + const ctx = await setupInlineEditor('hello world'); + try { + const range = ctx.editor.toDomRange({ index: 0, length: 5 }); + expect(range).not.toBeNull(); + setNativeSelection(range!); + + const preventDefault = vi.fn(); + const event = { + inputType: 'insertText', + data: 'x', + dataTransfer: null, + preventDefault, + stopPropagation: vi.fn(), + getTargetRanges: () => [ + { + startContainer: ctx.outside, + startOffset: 0, + endContainer: ctx.outside, + endOffset: 0, + }, + ], + } as unknown as InputEvent; + + await (ctx.editor.eventService as any)._onBeforeInput(event); + + expect(preventDefault).toHaveBeenCalledOnce(); + expect(ctx.editor.yTextString).toBe('x world'); + } finally { + await teardownInlineEditor(ctx); + } +}); diff --git a/blocksuite/framework/std/src/inline/services/event.ts b/blocksuite/framework/std/src/inline/services/event.ts index ee81a7a083..5765b31358 100644 --- a/blocksuite/framework/std/src/inline/services/event.ts +++ b/blocksuite/framework/std/src/inline/services/event.ts @@ -1,6 +1,7 @@ import { IS_ANDROID } from '@blocksuite/global/env'; import type { BaseTextAttributes } from '@blocksuite/store'; +import { INLINE_ROOT_ATTR } from '../consts.js'; import type { InlineEditor } from '../inline-editor.js'; import type { InlineRange } from '../types.js'; import { @@ -17,50 +18,121 @@ export class EventService { private _isComposing = false; + private readonly _getClosestInlineRoot = (node: Node): Element | null => { + const el = node instanceof Element ? node : node.parentElement; + return el?.closest(`[${INLINE_ROOT_ATTR}]`) ?? null; + }; + private readonly _isRangeCompletelyInRoot = (range: Range) => { if (range.commonAncestorContainer.ownerDocument !== document) return false; const rootElement = this.editor.rootElement; if (!rootElement) return false; - - const rootRange = document.createRange(); - rootRange.selectNode(rootElement); - - if ( - range.startContainer.compareDocumentPosition(range.endContainer) & - Node.DOCUMENT_POSITION_FOLLOWING - ) { - return ( - rootRange.comparePoint(range.startContainer, range.startOffset) >= 0 && - rootRange.comparePoint(range.endContainer, range.endOffset) <= 0 - ); - } else { - return ( - rootRange.comparePoint(range.endContainer, range.startOffset) >= 0 && - rootRange.comparePoint(range.startContainer, range.endOffset) <= 0 - ); - } + // Avoid `Range.comparePoint` here — Firefox/Chrome have subtle differences + // around selection points in `contenteditable` and comment marker nodes. + const containsStart = + range.startContainer === rootElement || + rootElement.contains(range.startContainer); + const containsEnd = + range.endContainer === rootElement || + rootElement.contains(range.endContainer); + return containsStart && containsEnd; }; private readonly _onBeforeInput = async (event: InputEvent) => { const range = this.editor.rangeService.getNativeRange(); - if ( - this.editor.isReadonly || - !range || - !this._isRangeCompletelyInRoot(range) - ) - return; + if (this.editor.isReadonly || !range) return; + const rootElement = this.editor.rootElement; + if (!rootElement) return; - let inlineRange = this.editor.toInlineRange(range); - if (!inlineRange) return; + const startInRoot = + range.startContainer === rootElement || + rootElement.contains(range.startContainer); + const endInRoot = + range.endContainer === rootElement || + rootElement.contains(range.endContainer); + + // Not this inline editor. + if (!startInRoot && !endInRoot) return; + + // If selection spans into another inline editor, let the range binding handle it. + if (startInRoot !== endInRoot) { + const otherNode = startInRoot ? range.endContainer : range.startContainer; + const otherRoot = this._getClosestInlineRoot(otherNode); + if (otherRoot && otherRoot !== rootElement) return; + } if (this._isComposing) { if (IS_ANDROID && event.inputType === 'insertCompositionText') { - this._compositionInlineRange = inlineRange; + const compositionInlineRange = this.editor.toInlineRange(range); + if (compositionInlineRange) { + this._compositionInlineRange = compositionInlineRange; + } } return; } + // Always prevent native DOM mutations inside inline editor. Browsers (notably + // Firefox) may remove Lit marker comment nodes during native edits, which + // will crash subsequent Lit updates with `ChildPart has no parentNode`. + event.preventDefault(); + + let inlineRange = this.editor.toInlineRange(range); + if (!inlineRange) { + // Some browsers may report selection points on non-text nodes inside + // `contenteditable`. Prefer the target range if available. + try { + const targetRanges = event.getTargetRanges(); + if (targetRanges.length > 0) { + const staticRange = targetRanges[0]; + const targetRange = document.createRange(); + targetRange.setStart( + staticRange.startContainer, + staticRange.startOffset + ); + targetRange.setEnd(staticRange.endContainer, staticRange.endOffset); + inlineRange = this.editor.toInlineRange(targetRange); + } + } catch { + // ignore + } + } + if (!inlineRange && startInRoot !== endInRoot) { + // Clamp a partially-outside selection to this editor so native editing + // won't touch Lit marker nodes. + const pointRange = document.createRange(); + if (startInRoot) { + pointRange.setStart(range.startContainer, range.startOffset); + pointRange.setEnd(range.startContainer, range.startOffset); + const startPoint = this.editor.toInlineRange(pointRange); + if (startPoint) { + inlineRange = { + index: startPoint.index, + length: this.editor.yTextLength - startPoint.index, + }; + } + } else { + pointRange.setStart(range.endContainer, range.endOffset); + pointRange.setEnd(range.endContainer, range.endOffset); + const endPoint = this.editor.toInlineRange(pointRange); + if (endPoint) { + inlineRange = { + index: 0, + length: endPoint.index, + }; + } + } + } + if (!inlineRange) { + // Try to recover from an unexpected DOM/selection state by rebuilding the + // editor DOM and retrying the range conversion. + this.editor.rerenderWholeEditor(); + await this.editor.waitForUpdate(); + const newRange = this.editor.rangeService.getNativeRange(); + inlineRange = newRange ? this.editor.toInlineRange(newRange) : null; + if (!inlineRange) return; + } + let ifHandleTargetRange = true; if ( @@ -88,15 +160,17 @@ export class EventService { range.setEnd(staticRange.endContainer, staticRange.endOffset); const targetInlineRange = this.editor.toInlineRange(range); - if (!isMaybeInlineRangeEqual(inlineRange, targetInlineRange)) { + // Ignore an un-resolvable target range to avoid swallowing the input. + if ( + targetInlineRange && + !isMaybeInlineRangeEqual(inlineRange, targetInlineRange) + ) { inlineRange = targetInlineRange; } } } if (!inlineRange) return; - event.preventDefault(); - if (IS_ANDROID) { this.editor.rerenderWholeEditor(); await this.editor.waitForUpdate(); diff --git a/tests/blocksuite/e2e/cross-platform/inline/firefox-beforeinput.spec.ts b/tests/blocksuite/e2e/cross-platform/inline/firefox-beforeinput.spec.ts new file mode 100644 index 0000000000..0ccb0c366f --- /dev/null +++ b/tests/blocksuite/e2e/cross-platform/inline/firefox-beforeinput.spec.ts @@ -0,0 +1,87 @@ +import { expect } from '@playwright/test'; + +import { + dragBetweenIndices, + enterPlaygroundRoom, + focusRichText, + getIndexCoordinate, + initEmptyParagraphState, + initThreeParagraphs, + pressForwardDelete, + type, + waitNextFrame, +} from '../../utils/actions/index.js'; +import { assertRichTexts, assertTextContain } from '../../utils/asserts.js'; +import { test } from '../../utils/playwright.js'; + +test('should not crash when deleting with selection dragged outside note (firefox)', async ({ + page, + browserName, +}) => { + test.skip(browserName !== 'firefox', 'Firefox-only regression'); + + await enterPlaygroundRoom(page); + await initEmptyParagraphState(page); + await initThreeParagraphs(page); + await assertRichTexts(page, ['123', '456', '789']); + + const outside = await page.evaluate(() => { + const note = document.querySelector('affine-note'); + if (!note) { + throw new Error('affine-note not found'); + } + const rect = note.getBoundingClientRect(); + return { x: rect.left + rect.width / 2, y: rect.bottom + 50 }; + }); + + // Drag starting from the end of the last paragraph to outside the note. + // Previously Firefox could perform native `contenteditable` deletion here and + // crash Lit updates with `ChildPart has no parentNode`. + await dragBetweenIndices( + page, + [2, 3], + [2, 3], + { x: 0, y: 0 }, + { x: 0, y: 0 }, + { + steps: 20, + async beforeMouseUp() { + await page.mouse.move(outside.x, outside.y); + }, + } + ); + + await pressForwardDelete(page); + await waitNextFrame(page); + await type(page, 'a'); + await waitNextFrame(page); + + await assertTextContain(page, 'a', 2); +}); + +test('should not crash when replacing the first word by double click (firefox)', async ({ + page, + browserName, +}) => { + test.skip(browserName !== 'firefox', 'Firefox-only regression'); + + await enterPlaygroundRoom(page); + await initEmptyParagraphState(page); + + await focusRichText(page, 0); + await type(page, 'hello world'); + await waitNextFrame(page); + + const coord = await getIndexCoordinate(page, [0, 1]); + await page.mouse.click(coord.x, coord.y, { clickCount: 2 }); + await type(page, 'x'); + await waitNextFrame(page); + + const text = await page.evaluate(() => { + const editorHost = document.querySelector('editor-host'); + const richText = editorHost?.querySelector('rich-text') as any; + return richText?.inlineEditor?.yText?.toString?.() ?? ''; + }); + expect(text.startsWith('x')).toBe(true); + expect(text.includes('world')).toBe(true); +});