diff --git a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts index 71bd54a89e7..5c68eb17acb 100644 --- a/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts +++ b/packages/ckeditor5-engine/src/view/observer/selectionobserver.ts @@ -11,6 +11,7 @@ import Observer from './observer.js'; import MutationObserver from './mutationobserver.js'; +import FocusObserver from './focusobserver.js'; import { env } from '@ckeditor/ckeditor5-utils'; import { debounce, type DebouncedFunc } from 'lodash-es'; @@ -18,7 +19,7 @@ import type View from '../view.js'; import type DocumentSelection from '../documentselection.js'; import type DomConverter from '../domconverter.js'; import type Selection from '../selection.js'; -import FocusObserver from './focusobserver.js'; +import type { ViewDocumentCompositionStartEvent } from './compositionobserver.js'; type DomSelection = globalThis.Selection; @@ -128,7 +129,7 @@ export default class SelectionObserver extends Observer { // Make sure that model selection is up-to-date at the end of selecting process. // Sometimes `selectionchange` events could arrive after the `mouseup` event and that selection could be already outdated. - this._handleSelectionChange( null, domDocument ); + this._handleSelectionChange( domDocument ); this.document.isSelecting = false; @@ -177,7 +178,7 @@ export default class SelectionObserver extends Observer { return; } - this._handleSelectionChange( domEvent, domDocument ); + this._handleSelectionChange( domDocument ); // @if CK_DEBUG_TYPING // if ( ( window as any ).logCKETyping ) { // @if CK_DEBUG_TYPING // console.groupEnd(); @@ -188,6 +189,11 @@ export default class SelectionObserver extends Observer { this._documentIsSelectingInactivityTimeoutDebounced(); } ); + // On composition start upcast the selection, so it includes composed text to be replaced on composition end. + this.listenTo( this.view.document, 'compositionstart', () => { + this._handleSelectionChange( domDocument ); + }, { priority: 'lowest' } ); + this._documents.add( domDocument ); } @@ -222,10 +228,9 @@ export default class SelectionObserver extends Observer { * a selection changes and fires {@link module:engine/view/document~Document#event:selectionChange} event on every change * and {@link module:engine/view/document~Document#event:selectionChangeDone} when a selection stop changing. * - * @param domEvent DOM event. * @param domDocument DOM document. */ - private _handleSelectionChange( domEvent: unknown, domDocument: Document ) { + private _handleSelectionChange( domDocument: Document ) { if ( !this.isEnabled ) { return; } diff --git a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js index 4c4c75359e2..31ff2e5c0c4 100644 --- a/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js +++ b/packages/ckeditor5-engine/tests/view/observer/selectionobserver.js @@ -18,6 +18,7 @@ import createViewRoot from '../_utils/createroot.js'; import { parse } from '../../../src/dev-utils/view.js'; import { StylesProcessor } from '../../../src/view/stylesmap.js'; import env from '@ckeditor/ckeditor5-utils/src/env.js'; +import { priorities } from '@ckeditor/ckeditor5-utils'; describe( 'SelectionObserver', () => { let view, viewDocument, viewRoot, selectionObserver, domRoot, domMain, domDocument; @@ -192,6 +193,48 @@ describe( 'SelectionObserver', () => { changeDomSelection(); } ); + it( 'should fire selectionChange synchronously on composition start event (at lowest priority)', () => { + let eventCount = 0; + let priorityCheck = 0; + + viewDocument.on( 'selectionChange', ( evt, data ) => { + expect( data ).to.have.property( 'domSelection' ).that.equals( domDocument.getSelection() ); + + expect( data ).to.have.property( 'oldSelection' ).that.is.instanceof( DocumentSelection ); + expect( data.oldSelection.rangeCount ).to.equal( 0 ); + + expect( data ).to.have.property( 'newSelection' ).that.is.instanceof( ViewSelection ); + expect( data.newSelection.rangeCount ).to.equal( 1 ); + + const newViewRange = data.newSelection.getFirstRange(); + const viewFoo = viewDocument.getRoot().getChild( 1 ).getChild( 0 ); + + expect( newViewRange.start.parent ).to.equal( viewFoo ); + expect( newViewRange.start.offset ).to.equal( 2 ); + expect( newViewRange.end.parent ).to.equal( viewFoo ); + expect( newViewRange.end.offset ).to.equal( 2 ); + + expect( priorityCheck ).to.equal( 1 ); + + eventCount++; + } ); + + viewDocument.on( 'compositionstart', () => { + priorityCheck++; + }, { priority: priorities.lowest + 1 } ); + + viewDocument.on( 'compositionstart', () => { + priorityCheck++; + }, { priority: priorities.lowest - 1 } ); + + changeDomSelection(); + + viewDocument.fire( 'compositionstart' ); + + expect( eventCount ).to.equal( 1 ); + expect( priorityCheck ).to.equal( 2 ); + } ); + it( 'should not fire selectionChange for ignored target', done => { viewDocument.on( 'selectionChange', () => { throw 'selectionChange fired in ignored elements'; diff --git a/packages/ckeditor5-typing/src/input.ts b/packages/ckeditor5-typing/src/input.ts index 7716904fa02..941b683dde6 100644 --- a/packages/ckeditor5-typing/src/input.ts +++ b/packages/ckeditor5-typing/src/input.ts @@ -13,7 +13,11 @@ import { env } from '@ckeditor/ckeditor5-utils'; import InsertTextCommand from './inserttextcommand.js'; import InsertTextObserver, { type ViewDocumentInsertTextEvent } from './inserttextobserver.js'; -import type { Model } from '@ckeditor/ckeditor5-engine'; +import { + type Model, + type ViewDocumentCompositionStartEvent, + type ViewDocumentKeyDownEvent +} from '@ckeditor/ckeditor5-engine'; /** * Handles text input coming from the keyboard or other input methods. @@ -53,10 +57,17 @@ export default class Input extends Plugin { const { text, selection: viewSelection, resultRange: viewResultRange } = data; + let modelRanges; + // If view selection was specified, translate it to model selection. - const modelRanges = Array.from( viewSelection.getRanges() ).map( viewRange => { - return editor.editing.mapper.toModelRange( viewRange ); - } ); + if ( viewSelection ) { + modelRanges = Array.from( viewSelection.getRanges() ).map( viewRange => { + return editor.editing.mapper.toModelRange( viewRange ); + } ); + } + else { + modelRanges = Array.from( modelSelection.getRanges() ); + } let insertText = text; @@ -109,7 +120,7 @@ export default class Input extends Plugin { // On Android with English keyboard, the composition starts just by putting caret // at the word end or by selecting a table column. This is not a real composition started. // Trigger delete content on first composition key pressed. - this.listenTo( view.document, 'keydown', ( evt, data ) => { + this.listenTo( view.document, 'keydown', ( evt, data ) => { if ( modelSelection.isCollapsed || data.keyCode != 229 || !view.document.isComposing ) { return; } @@ -129,7 +140,7 @@ export default class Input extends Plugin { } else { // Note: The priority must precede the CompositionObserver handler to call it before // the renderer is blocked, because we want to render this change. - this.listenTo( view.document, 'compositionstart', () => { + this.listenTo( view.document, 'compositionstart', () => { if ( modelSelection.isCollapsed ) { return; } diff --git a/packages/ckeditor5-typing/src/inserttextobserver.ts b/packages/ckeditor5-typing/src/inserttextobserver.ts index 3a94c18da04..dfb7e070cb2 100644 --- a/packages/ckeditor5-typing/src/inserttextobserver.ts +++ b/packages/ckeditor5-typing/src/inserttextobserver.ts @@ -128,8 +128,7 @@ export default class InsertTextObserver extends Observer { // `beforeinput` would come with just the range it's changing and we'd need to calculate that. // We decided to go with the 2nd option for its simplicity and stability. viewDocument.fire( 'insertText', new DomEventData( view, domEvent, { - text: data, - selection: viewDocument.selection + text: data } ) ); }, { priority: 'lowest' } ); } diff --git a/packages/ckeditor5-typing/tests/input.js b/packages/ckeditor5-typing/tests/input.js index ffd325fa73e..73c6465ce64 100644 --- a/packages/ckeditor5-typing/tests/input.js +++ b/packages/ckeditor5-typing/tests/input.js @@ -3,11 +3,12 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* global document */ +/* global document, window */ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor.js'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils.js'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph.js'; +import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata.js'; import Input from '../src/input.js'; import InsertTextCommand from '../src/inserttextcommand.js'; @@ -127,6 +128,28 @@ describe( 'Input', () => { expect( firstCallArgs.resultRange ).to.be.undefined; } ); + it( 'should use model document selection if the selection property is not passed', async () => { + const expectedSelection = editor.model.createSelection( + editor.model.createPositionAt( editor.model.document.getRoot().getChild( 0 ), 1 ) + ); + + editor.model.change( writer => { + writer.setSelection( expectedSelection ); + } ); + + viewDocument.fire( 'insertText', { + text: 'bar', + preventDefault: sinon.spy() + } ); + + const firstCallArgs = insertTextCommandSpy.firstCall.args[ 0 ]; + + sinon.assert.calledOnce( insertTextCommandSpy ); + expect( firstCallArgs.text ).to.equal( 'bar' ); + expect( firstCallArgs.selection.isEqual( expectedSelection ) ).to.be.true; + expect( firstCallArgs.resultRange ).to.be.undefined; + } ); + it( 'should have result range passed correctly to the insert text command', async () => { const expectedSelection = editor.model.createSelection( editor.model.createPositionAt( editor.model.document.getRoot().getChild( 0 ), 1 ) @@ -169,6 +192,40 @@ describe( 'Input', () => { sinon.assert.calledWithExactly( spy, editor.model.document.selection ); } ); + it( 'should update model selection to the DOM selection on composition start and use it on compositionend', () => { + const root = editor.model.document.getRoot(); + const modelSelection = editor.model.document.selection; + + const modelParagraph = root.getChild( 0 ); + const viewParagraph = viewDocument.getRoot().getChild( 0 ); + const domParagraph = view.domConverter.mapViewToDom( viewParagraph ); + + const expectedRange = editor.model.createRange( + editor.model.createPositionAt( modelParagraph, 1 ), + editor.model.createPositionAt( modelParagraph, 3 ) + ); + + editor.model.change( writer => writer.setSelection( root.getChild( 0 ), 0 ) ); + + window.getSelection().setBaseAndExtent( domParagraph.childNodes[ 0 ], 1, domParagraph.childNodes[ 0 ], 3 ); + + viewDocument.fire( 'compositionstart' ); + + expect( modelSelection.getFirstRange().isEqual( expectedRange ) ).to.be.true; + + viewDocument.fire( 'compositionend', new DomEventData( view, { + preventDefault() {} + }, { + data: 'bar' + } ) ); + + const firstCallArgs = insertTextCommandSpy.firstCall.args[ 0 ]; + + sinon.assert.calledOnce( insertTextCommandSpy ); + expect( firstCallArgs.text ).to.equal( 'bar' ); + expect( firstCallArgs.selection.getFirstRange().isEqual( expectedRange ) ).to.be.true; + } ); + it( 'should not call model.deleteContent() on composition start for collapsed model selection', () => { const spy = sinon.spy( editor.model, 'deleteContent' ); const root = editor.model.document.getRoot(); diff --git a/packages/ckeditor5-typing/tests/inserttextobserver.js b/packages/ckeditor5-typing/tests/inserttextobserver.js index 7ff945192d6..0f8cfdf9881 100644 --- a/packages/ckeditor5-typing/tests/inserttextobserver.js +++ b/packages/ckeditor5-typing/tests/inserttextobserver.js @@ -186,7 +186,7 @@ describe( 'InsertTextObserver', () => { const firstCallArgs = insertTextEventSpy.firstCall.args[ 1 ]; expect( firstCallArgs.text ).to.equal( 'bar' ); - expect( firstCallArgs.selection.isEqual( view.document.selection ) ).to.be.true; + expect( firstCallArgs.selection ).to.be.undefined; } ); it( 'should ignore the empty compositionend event (without any data)', () => {