Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix caret jumping in certain cases with cursor blot #4265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 34 additions & 22 deletions packages/quill/src/core/selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,18 +116,7 @@ class Selection {
});
this.emitter.on(Emitter.events.COMPOSITION_END, () => {
this.composing = false;
if (this.cursor.parent) {
const range = this.cursor.restore();
if (!range) return;
setTimeout(() => {
this.setNativeRange(
range.startNode,
range.startOffset,
range.endNode,
range.endOffset,
);
}, 1);
}
this.restoreCursor();
});
}

Expand Down Expand Up @@ -444,20 +433,11 @@ class Selection {
}
if (!isEqual(oldRange, this.lastRange)) {
if (
!this.composing &&
nativeRange != null &&
nativeRange.native.collapsed &&
nativeRange.start.node !== this.cursor.textNode
) {
const range = this.cursor.restore();
if (range) {
this.setNativeRange(
range.startNode,
range.startOffset,
range.endNode,
range.endOffset,
);
}
this.restoreCursor();
}
const args = [
Emitter.events.SELECTION_CHANGE,
Expand All @@ -471,6 +451,38 @@ class Selection {
}
}
}

private restoreCursor() {
if (!this.cursor.parent || this.composing) return;

const restoredRange = this.cursor.restore();
if (restoredRange) {
this.setNativeRange(
restoredRange.startNode,
restoredRange.startOffset,
restoredRange.endNode,
restoredRange.endOffset,
);
}
this.emitter.once(Emitter.events.SCROLL_OPTIMIZE, () => {
// Re-apply the selection after the optimize phase.
// This is needed in two cases:
// 1. When `restoredRange` is null: this can happen when the caret
// is moved out of the cursor blot by the user.
// 2. When `restoredRange` is not null but either point of it does not
// exist in the DOM tree anymore: this can happen when the cursor blot
// is empty (e.g. when user cancels a composition session) so that the
// wrapping empty blot referred by `restoredRange` is removed in the
// optimize phase.
if (
!restoredRange ||
!this.root.contains(restoredRange.startNode) ||
(restoredRange.endNode && !this.root.contains(restoredRange.endNode))
) {
this.setRange(this.lastRange);
}
});
}
}

function contains(parent: Node, descendant: Node) {
Expand Down
137 changes: 137 additions & 0 deletions packages/quill/test/e2e/cursor.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import { expect } from '@playwright/test';
import { test } from './fixtures/index.js';
import { SHORTKEY } from './utils/index.js';

test.describe('cursor', () => {
test.beforeEach(async ({ editorPage }) => {
await editorPage.open();
});

test.describe('type', () => {
test('normal typing with one format', async ({ page, editorPage }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.type('abc');
expect(await editorPage.getContents()).toEqual([
{ insert: '12' },
{ insert: 'abc', attributes: { bold: true } },
{ insert: '34\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 5, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('normal typing with two formats', async ({ page, editorPage }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.press(`${SHORTKEY}+i`);
await page.keyboard.type('abc');
expect(await editorPage.getContents()).toEqual([
{ insert: '12' },
{ insert: 'abc', attributes: { bold: true, italic: true } },
{ insert: '34\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 5, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('normal typing with one format omitting', async ({
page,
editorPage,
}) => {
await editorPage.setContents([
{ insert: '1234', attributes: { bold: true, italic: true } },
{ insert: '\n' },
]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.type('abc');
expect(await editorPage.getContents()).toEqual([
{ insert: '12', attributes: { bold: true, italic: true } },
{ insert: 'abc', attributes: { italic: true } },
{ insert: '34', attributes: { bold: true, italic: true } },
{ insert: '\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 5, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});
});

test('paste', async ({ clipboard, editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await clipboard.writeText('abc');
await page.keyboard.press(`${SHORTKEY}+b`);
await clipboard.paste();
expect(await editorPage.getContents()).toEqual([
{ insert: '12' },
{ insert: 'abc', attributes: { bold: true } },
{ insert: '34\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 5, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test.describe('IME', () => {
test('confirm composition', async ({ composition, editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
const ime = await composition.start();
await ime.update('w');
await ime.update('o');
await ime.commit('我');
expect(await editorPage.getContents()).toEqual([
{ insert: '12' },
{ insert: '我', attributes: { bold: true } },
{ insert: '34\n' },
]);
expect(await editorPage.getSelection()).toEqual({ index: 3, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('cancel composition', async ({ composition, editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
const ime = await composition.start();
await ime.update('w');
await ime.update('o');
await ime.cancel();
expect(await editorPage.getContents()).toEqual([{ insert: '1234\n' }]);
expect(await editorPage.getSelection()).toEqual({ index: 2, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});
});

test.describe('caret movements', () => {
test('right arrow key', async ({ editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.press('ArrowRight');
expect(await editorPage.getSelection()).toEqual({ index: 3, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('left arrow key', async ({ editorPage, page }) => {
await editorPage.setContents([{ insert: '1234\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await page.keyboard.press('ArrowLeft');
expect(await editorPage.getSelection()).toEqual({ index: 1, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});

test('jumping', async ({ editorPage, page }) => {
await editorPage.setContents([{ insert: '12345\n' }]);
await editorPage.moveCursorAfterText('12');
await page.keyboard.press(`${SHORTKEY}+b`);
await editorPage.moveCursorAfterText('34');
expect(await editorPage.getSelection()).toEqual({ index: 4, length: 0 });
await expect(editorPage.cursorBlot).not.toBeAttached();
});
});
});
23 changes: 17 additions & 6 deletions packages/quill/test/e2e/fixtures/Composition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
abstract class CompositionSession {
abstract update(key: string): Promise<void>;
abstract commit(committedText: string): Promise<void>;
abstract cancel(): Promise<void>;

protected composingData = '';

Expand Down Expand Up @@ -35,12 +36,7 @@ class ChromiumCompositionSession extends CompositionSession {
async update(key: string) {
await this.withKeyboardEvents(key, async () => {
this.composingData += key;

await this.session.send('Input.imeSetComposition', {
selectionStart: this.composingData.length,
selectionEnd: this.composingData.length,
text: this.composingData,
});
this.updateComposition();
});
}

Expand All @@ -51,6 +47,21 @@ class ChromiumCompositionSession extends CompositionSession {
});
});
}

async cancel() {
await this.withKeyboardEvents('Escape', async () => {
this.composingData = '';
await this.updateComposition();
});
}

private async updateComposition() {
await this.session.send('Input.imeSetComposition', {
selectionStart: this.composingData.length,
selectionEnd: this.composingData.length,
text: this.composingData,
});
}
}

class Composition {
Expand Down
4 changes: 4 additions & 0 deletions packages/quill/test/e2e/pageobjects/EditorPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ export default class EditorPage {
return this.page.locator('.ql-editor');
}

get cursorBlot() {
return this.root.locator('.ql-cursor');
}

async open() {
await this.page.goto('/');
await this.page.waitForSelector('.ql-editor', { timeout: 10000 });
Expand Down
Loading