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.

Sign up for GitHub

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

The insertText event and related beforeinput events should not be canceled #17566

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

The insertText event and related beforeinput events should not be canceled #17566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3aa035c
The insertText event and related beforeinput events should not be can…
niegowski Nov 29, 2024
6e6a200
Improved handling of multi-line text replacements.
niegowski Nov 29, 2024
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
4 changes: 2 additions & 2 deletions packages/ckeditor5-engine/src/view/observer/compositionobserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default class CompositionObserver extends DomEventObserver<'compositionst
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
document.isComposing = true;
}, { priority: 'low' } );
} );

document.on<ViewDocumentCompositionEndEvent>( 'compositionend', () => {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
Expand All @@ -52,7 +52,7 @@ export default class CompositionObserver extends DomEventObserver<'compositionst
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
document.isComposing = false;
}, { priority: 'low' } );
} );
}

/**
Expand Down
17 changes: 8 additions & 9 deletions packages/ckeditor5-engine/src/view/observer/inputobserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {

// Normalize the insertText data that includes new-line characters.
// https://github.com/ckeditor/ckeditor5/issues/2045.
if ( domEvent.inputType == 'insertText' && data && data.includes( '\n' ) ) {
if ( [ 'insertText', 'insertReplacementText' ].includes( domEvent.inputType ) && data && data.includes( '\n' ) ) {
// There might be a single new-line or double for new paragraph, but we translate
// it to paragraphs as it is our default action for enter handling.
const parts = data.split( /\n{1,2}/g );
Expand All @@ -159,15 +159,14 @@ export default class InputObserver extends DomEventObserver<'beforeinput'> {
partTargetRanges = [ viewDocument.selection.getFirstRange()! ];
}

if ( i + 1 < parts.length ) {
this.fire( domEvent.type, domEvent, {
inputType: 'insertParagraph',
targetRanges: partTargetRanges
} );
// After a batch of paragraphs interleaved with text fire a dummy beforeinput just to flush the input queue.
this.fire( domEvent.type, domEvent, {
inputType: i + 1 < parts.length ? 'insertParagraph' : 'dummy',
targetRanges: partTargetRanges
} );

// Use the result view selection so following events will be added one after another.
partTargetRanges = [ viewDocument.selection.getFirstRange()! ];
}
// Use the result view selection so following events will be added one after another.
partTargetRanges = [ viewDocument.selection.getFirstRange()! ];
}

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
Expand Down
144 changes: 64 additions & 80 deletions packages/ckeditor5-typing/src/input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import {
type ViewDocumentCompositionStartEvent,
type ViewDocumentCompositionEndEvent,
type ViewDocumentKeyDownEvent,
type ViewDocumentMutationsEvent
type ViewDocumentMutationsEvent,
type ViewDocumentInputEvent
} from '@ckeditor/ckeditor5-engine';

import { debounce } from 'lodash-es';
Expand Down Expand Up @@ -74,19 +75,13 @@ export default class Input extends Plugin {
editor.commands.add( 'insertText', insertTextCommand );
editor.commands.add( 'input', insertTextCommand );

this.listenTo<ViewDocumentInsertTextEvent>( view.document, 'insertText', ( evt, data ) => {
// Rendering is disabled while composing so prevent events that will be rendered by the engine
// and should not be applied by the browser.
if ( !view.document.isComposing ) {
data.preventDefault();
}

this.listenTo<ViewDocumentInputEvent>( view.document, 'beforeinput', () => {
// Flush queue on the next beforeinput event because it could happen
// that the mutation observer does not notice the DOM change in time.
if ( env.isAndroid && view.document.isComposing ) {
this._compositionQueue.flush( 'next beforeinput' );
}
this._compositionQueue.flush( 'next beforeinput' );
}, { priority: 'high' } );

this.listenTo<ViewDocumentInsertTextEvent>( view.document, 'insertText', ( evt, data ) => {
const { text, selection: viewSelection } = data;

let modelRanges;
Expand Down Expand Up @@ -139,33 +134,25 @@ export default class Input extends Plugin {
selection: model.createSelection( modelRanges )
};

// This is a composition event and those are not cancellable, so we need to wait until browser updates the DOM
// This is a beforeinput event, so we need to wait until browser updates the DOM
// and we could apply changes to the model and verify if the DOM is valid.
// The browser applies changes to the DOM not immediately on beforeinput event.
// We just wait for mutation observer to notice changes or as a fallback a timeout.
if ( env.isAndroid && view.document.isComposing ) {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.log( `%c[Input]%c Queue insertText:%c "${ commandData.text }"%c ` +
// @if CK_DEBUG_TYPING // `[${ commandData.selection.getFirstPosition().path }]-` +
// @if CK_DEBUG_TYPING // `[${ commandData.selection.getLastPosition().path }]` +
// @if CK_DEBUG_TYPING // ` queue size: ${ this._compositionQueue.length + 1 }`,
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green;', 'font-weight: bold', 'color: blue', ''
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
//
// Previously we were cancelling the non-composition events, but it caused issues especially in Safari.

this._compositionQueue.push( commandData );
} else {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.log( `%c[Input]%c Execute insertText:%c "${ commandData.text }"%c ` +
// @if CK_DEBUG_TYPING // `[${ commandData.selection.getFirstPosition().path }]-` +
// @if CK_DEBUG_TYPING // `[${ commandData.selection.getLastPosition().path }]`,
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green;', 'font-weight: bold', 'color: blue', ''
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.log( `%c[Input]%c Queue insertText:%c "${ commandData.text }"%c ` +
// @if CK_DEBUG_TYPING // `[${ commandData.selection.getFirstPosition().path }]-` +
// @if CK_DEBUG_TYPING // `[${ commandData.selection.getLastPosition().path }]` +
// @if CK_DEBUG_TYPING // ` queue size: ${ this._compositionQueue.length + 1 }`,
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green;', 'font-weight: bold', 'color: blue', ''
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }

editor.execute( 'insertText', commandData );
view.scrollToTheSelection();
}
// TODO verify if this work correctly in WTA and table selection.

this._compositionQueue.push( commandData );
} );

// Delete selected content on composition start.
Expand Down Expand Up @@ -209,19 +196,15 @@ export default class Input extends Plugin {
// @if CK_DEBUG_TYPING // }

deleteSelectionContent( model, insertTextCommand );
} );
}, { priority: 'high' } );
}

// Apply composed changes to the model.
if ( env.isAndroid ) {
// Apply changes to the model as they are applied to the DOM by the browser.
// On beforeinput event, the DOM is not yet modified. We wait for detected mutations to apply model changes.
this.listenTo<ViewDocumentMutationsEvent>( view.document, 'mutations', ( evt, { mutations } ) => {
if ( !view.document.isComposing ) {
return;
}

// Check if mutations are relevant for queued changes.
// Apply changes to the model as they are applied to the DOM by the browser.
// On beforeinput event, the DOM is not yet modified. We wait for detected mutations to apply model changes.
this.listenTo<ViewDocumentMutationsEvent>( view.document, 'mutations', ( evt, { mutations } ) => {
// Check if mutations are relevant for queued changes.
if ( this._compositionQueue.hasComposedElements() ) {
for ( const { node } of mutations ) {
const viewElement = findMappedViewAncestor( node, mapper );
const modelElement = mapper.toModelElement( viewElement )!;
Expand All @@ -232,27 +215,32 @@ export default class Input extends Plugin {
return;
}
}
}

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.log( '%c[Input]%c Mutations not related to the composition.',
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green;', 'font-style: italic'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
} );
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.log( '%c[Input]%c Mutations not related to the composition.',
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green;', 'font-style: italic'
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }
} );

// Make sure that all changes are applied to the model before the end of composition.
this.listenTo<ViewDocumentCompositionEndEvent>( view.document, 'compositionend', () => {
this._compositionQueue.flush( 'composition end' );
} );
// Make sure that all changes are applied to the model before the end of composition.
this.listenTo<ViewDocumentCompositionEndEvent>( view.document, 'compositionend', () => {
this._compositionQueue.flush( 'before composition end' );
}, { priority: 'high' } );

// Trigger mutations check after the composition completes to fix all DOM changes that got ignored during composition.
// On Android the Renderer is not disabled while composing. While updating DOM nodes we ignore some changes
// that are not that important (like NBSP vs plain space character) and could break the composition flow.
// After composition is completed we trigger additional `mutations` event for elements affected by the composition
// so the Renderer can adjust the DOM to the expected structure without breaking the composition.
this.listenTo<ViewDocumentCompositionEndEvent>( view.document, 'compositionend', () => {
// There could be new item queued on composition end so flush it.
this._compositionQueue.flush( 'after composition end' );

// Trigger mutations check after the composition completes to fix all DOM changes that got ignored during composition.
// On Android the Renderer is not disabled while composing. While updating DOM nodes we ignore some changes
// that are not that important (like NBSP vs plain space character) and could break the composition flow.
// After composition is completed we trigger additional `mutations` event for elements affected by the composition
// so the Renderer can adjust the DOM to the expected structure without breaking the composition.
this.listenTo<ViewDocumentCompositionEndEvent>( view.document, 'compositionend', () => {
const mutations: Array<MutationData> = [];
const mutations: Array<MutationData> = [];

if ( this._compositionQueue.hasComposedElements() ) {
for ( const element of this._compositionQueue.flushComposedElements() ) {
const viewElement = mapper.toViewElement( element );

Expand All @@ -262,22 +250,11 @@ export default class Input extends Plugin {

mutations.push( { type: 'children', node: viewElement } );
}
}

if ( mutations.length ) {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.group( '%c[Input]%c Fire post-composition mutation fixes.',
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green', 'font-weight: bold', ''
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }

view.document.fire<ViewDocumentMutationsEvent>( 'mutations', { mutations } );

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.groupEnd();
// @if CK_DEBUG_TYPING // }
}
}, { priority: 'lowest' } );
} else {
// Fire composition mutations if any.
//
// For non-Android:
// After composition end we need to verify if there are no left-overs.
// Listening at the lowest priority so after the `InsertTextObserver` added above (all composed text
// should already be applied to the model, view, and DOM).
Expand All @@ -289,20 +266,20 @@ export default class Input extends Plugin {
// It in the most cases just clears the internal record of mutated text nodes
// since all changes should already be applied to the DOM.
// This is especially needed when user cancels composition, so we can clear nodes marked to sync.
this.listenTo<ViewDocumentCompositionEndEvent>( view.document, 'compositionend', () => {
if ( mutations.length || !env.isAndroid ) {
// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.group( '%c[Input]%c Force render after composition end.',
// @if CK_DEBUG_TYPING // console.group( '%c[Input]%c Fire post-composition mutation fixes.',
// @if CK_DEBUG_TYPING // 'font-weight: bold; color: green', 'font-weight: bold', ''
// @if CK_DEBUG_TYPING // );
// @if CK_DEBUG_TYPING // }

view.document.fire<ViewDocumentMutationsEvent>( 'mutations', { mutations: [] } );
view.document.fire<ViewDocumentMutationsEvent>( 'mutations', { mutations } );

// @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) {
// @if CK_DEBUG_TYPING // console.groupEnd();
// @if CK_DEBUG_TYPING // }
}, { priority: 'lowest' } );
}
}
}, { priority: 'lowest' } );
}

/**
Expand Down Expand Up @@ -470,6 +447,13 @@ class CompositionQueue {
return this._compositionElements.has( element );
}

/**
* TODO
*/
public hasComposedElements(): boolean {
return this._compositionElements.size > 0;
}

/**
* Returns an array of composition-related elements and clears the internal list.
*/
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-typing/src/inserttextobserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export default class InsertTextObserver extends Observer {
viewDocument.fire( 'insertText', new DomEventData( view, domEvent, {
text: data
} ) );
}, { priority: 'lowest' } );
}, { priority: 'low' } );
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-widget/src/widgettypearound/widgettypearound.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ export default class WidgetTypeAround extends Plugin {
// Note: The priority must precede the default Input plugin compositionstart handler (to call it before delete content).
this._listenToIfEnabled<ViewDocumentCompositionStartEvent>( viewDocument, 'compositionstart', () => {
this._insertParagraphAccordingToFakeCaretPosition();
}, { priority: 'high' } );
}, { priority: 'highest' } );
}
}

Expand Down