Added fixer for the beforeInput target ranges, so they make sense aft…

…er mapping to the model.
ckeditor · Mar 28, 2023 · f6c1b6b · f6c1b6b
1 parent f9f2d13
commit f6c1b6b
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 3 deletions.
diff --git a/packages/ckeditor5-engine/src/controller/editingcontroller.ts b/packages/ckeditor5-engine/src/controller/editingcontroller.ts
@@ -7,7 +7,11 @@
  * @module engine/controller/editingcontroller
  */
 
-import { CKEditorError, ObservableMixin } from '@ckeditor/ckeditor5-utils';
+import {
+	CKEditorError,
+	ObservableMixin,
+	type GetCallback
+} from '@ckeditor/ckeditor5-utils';
 
 import RootEditableElement from '../view/rooteditableelement';
 import View from '../view/view';
@@ -28,14 +32,18 @@ import {
 
 import { convertSelectionChange } from '../conversion/upcasthelpers';
 
+import { tryFixingRange } from '../model/utils/selection-post-fixer';
+
 import type { default as Model, AfterChangesEvent, BeforeChangesEvent } from '../model/model';
 import type ModelItem from '../model/item';
 import type ModelText from '../model/text';
 import type ModelTextProxy from '../model/textproxy';
+import type Schema from '../model/schema';
 import type { DocumentChangeEvent } from '../model/document';
 import type { Marker } from '../model/markercollection';
 import type { StylesProcessor } from '../view/stylesmap';
 import type { ViewDocumentSelectionChangeEvent } from '../view/observer/selectionobserver';
+import type { ViewDocumentInputEvent } from '../view/observer/inputobserver';
 
 // @if CK_DEBUG_ENGINE // const { dumpTrees, initDocumentDumping } = require( '../dev-utils/utils' );
 
@@ -115,6 +123,11 @@ export default class EditingController extends ObservableMixin() {
 			convertSelectionChange( this.model, this.mapper )
 		);
 
+		this.listenTo<ViewDocumentInputEvent>( this.view.document, 'beforeinput',
+			fixTargetRanges( this.mapper, this.model.schema ),
+			{ priority: 'high' }
+		);
+
 		// Attach default model converters.
 		this.downcastDispatcher.on<DowncastInsertEvent<ModelText | ModelTextProxy>>( 'insert:$text', insertText(), { priority: 'lowest' } );
 		this.downcastDispatcher.on<DowncastInsertEvent>( 'insert', insertAttributesAndChildren(), { priority: 'lowest' } );
@@ -232,3 +245,26 @@ export default class EditingController extends ObservableMixin() {
 		} );
 	}
 }
+
+/**
+ * TODO
+ */
+function fixTargetRanges( mapper: Mapper, schema: Schema ): GetCallback<ViewDocumentInputEvent> {
+	return ( evt, data ) => {
+		if ( !data.targetRanges ) {
+			return;
+		}
+
+		for ( let i = 0; i < data.targetRanges.length; i++ ) {
+			const viewRange = data.targetRanges[ i ];
+			const modelRange = mapper.toModelRange( viewRange );
+			const correctedRange = tryFixingRange( modelRange, schema );
+
+			if ( !correctedRange || correctedRange.isEqual( modelRange ) ) {
+				continue;
+			}
+
+			data.targetRanges[ i ] = mapper.toViewRange( correctedRange );
+		}
+	};
+}
diff --git a/packages/ckeditor5-engine/src/conversion/mapper.ts b/packages/ckeditor5-engine/src/conversion/mapper.ts
@@ -518,6 +518,7 @@ export default class Mapper extends EmitterMixin() {
 
 		// If the position is a text it is simple ("ba|r" -> 2).
 		if ( viewParent.is( '$text' ) ) {
+			// TODO throw if viewOffset is bigger than text node length? But this would explode in IME.
 			return viewOffset;
 		}
 

diff --git a/packages/ckeditor5-engine/src/model/utils/selection-post-fixer.ts b/packages/ckeditor5-engine/src/model/utils/selection-post-fixer.ts
@@ -116,7 +116,7 @@ function selectionPostFixer( writer: Writer, model: Model ): boolean {
  *
  * @returns Returns fixed range or null if range is valid.
  */
-function tryFixingRange( range: Range, schema: Schema ) {
+export function tryFixingRange( range: Range, schema: Schema ): Range | null {
 	if ( range.isCollapsed ) {
 		return tryFixingCollapsedRange( range, schema );
 	}

diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts
@@ -852,6 +852,7 @@ export default class DomConverter {
 				offset = offset < 0 ? 0 : offset;
 			}
 
+			// TODO throw or return null if offset is bigger than text node length? But this would explode in IME.
 			return new ViewPosition( viewParent, offset );
 		}
 		// domParent instanceof HTMLElement.
@@ -865,7 +866,8 @@ export default class DomConverter {
 			} else {
 				const domBefore = domParent.childNodes[ domOffset - 1 ];
 
-				if ( isText( domBefore ) && isInlineFiller( domBefore ) ) {
+				// Jump over an inline filler (and also on Firefox jump over a block filler while pressing backspace in an empty paragraph).
+				if ( isText( domBefore ) && isInlineFiller( domBefore ) || this.isBlockFiller( domBefore ) ) {
 					return this.domPositionToView( domBefore.parentNode!, indexOf( domBefore ) );
 				}