mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-02-04 08:38:34 +00:00
fix: firefox input (#14315)
fix #14296 fix #14289 #### PR Dependency Tree * **PR #14315** 👈 This tree was auto-generated by [Charcoal](https://github.com/danerwilliams/charcoal) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
33
.github/workflows/build-test.yml
vendored
33
.github/workflows/build-test.yml
vendored
@@ -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: |
|
||||
|
||||
@@ -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<ReturnType<typeof setupInlineEditor>>
|
||||
) {
|
||||
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);
|
||||
}
|
||||
});
|
||||
@@ -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<TextAttributes extends BaseTextAttributes> {
|
||||
|
||||
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<TextAttributes extends BaseTextAttributes> {
|
||||
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();
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user