Merge pull request #15012 from ckeditor/ck/14709-prevent-dropping-in-…

…disallowed-places Fix (clipboard): Improved drop marker visibility to only display in permissible drop locations. Closes #14709.
ckeditor · Oct 11, 2023 · 2833131 · 2833131
2 parents c3fc28a + 521aa6e
commit 2833131
Show file tree

Hide file tree

Showing 4 changed files with 303 additions and 26 deletions.
diff --git a/packages/ckeditor5-clipboard/src/dragdrop.ts b/packages/ckeditor5-clipboard/src/dragdrop.ts
@@ -345,7 +345,14 @@ export default class DragDrop extends Plugin {
 
 			const { clientX, clientY } = ( data as DomEventData<DragEvent> ).domEvent;
 
-			dragDropTarget.updateDropMarker( data.target, data.targetRanges, clientX, clientY, this._blockMode );
+			dragDropTarget.updateDropMarker(
+				data.target,
+				data.targetRanges,
+				clientX,
+				clientY,
+				this._blockMode,
+				this._draggedRange
+			);
 
 			// If this is content being dragged from another editor, moving out of current editor instance
 			// is not possible until 'dragend' event case will be fixed.
@@ -382,7 +389,14 @@ export default class DragDrop extends Plugin {
 			}
 
 			const { clientX, clientY } = ( data as DomEventData<DragEvent> ).domEvent;
-			const targetRange = dragDropTarget.getFinalDropRange( data.target, data.targetRanges, clientX, clientY, this._blockMode );
+			const targetRange = dragDropTarget.getFinalDropRange(
+				data.target,
+				data.targetRanges,
+				clientX,
+				clientY,
+				this._blockMode,
+				this._draggedRange
+			);
 
 			if ( !targetRange ) {
 				this._finalizeDragging( false );

diff --git a/packages/ckeditor5-clipboard/src/dragdroptarget.ts b/packages/ckeditor5-clipboard/src/dragdroptarget.ts
@@ -13,8 +13,10 @@ import {
 } from '@ckeditor/ckeditor5-core';
 
 import {
+	type Node,
 	type Element,
 	type Range,
+	type LiveRange,
 	type ViewElement,
 	type ViewRange,
 	type DowncastWriter,
@@ -122,16 +124,32 @@ export default class DragDropTarget extends Plugin {
 		targetViewRanges: Array<ViewRange> | null,
 		clientX: number,
 		clientY: number,
-		blockMode: boolean
+		blockMode: boolean,
+		draggedRange: LiveRange | null
 	): void {
 		this.removeDropMarkerDelayed.cancel();
 
-		const targetRange = findDropTargetRange( this.editor, targetViewElement, targetViewRanges, clientX, clientY, blockMode );
+		const targetRange = findDropTargetRange(
+			this.editor,
+			targetViewElement,
+			targetViewRanges,
+			clientX,
+			clientY,
+			blockMode,
+			draggedRange
+		);
+
+		/* istanbul ignore next -- @preserve */
+		if ( !targetRange ) {
+			return;
+		}
 
-		/* istanbul ignore else -- @preserve */
-		if ( targetRange ) {
-			this._updateDropMarkerThrottled( targetRange );
+		if ( draggedRange && draggedRange.containsRange( targetRange ) ) {
+			// Target range is inside the dragged range.
+			return this.removeDropMarker();
 		}
+
+		this._updateDropMarkerThrottled( targetRange );
 	}
 
 	/**
@@ -144,9 +162,18 @@ export default class DragDropTarget extends Plugin {
 		targetViewRanges: Array<ViewRange> | null,
 		clientX: number,
 		clientY: number,
-		blockMode: boolean
+		blockMode: boolean,
+		draggedRange: LiveRange | null
 	): Range | null {
-		const targetRange = findDropTargetRange( this.editor, targetViewElement, targetViewRanges, clientX, clientY, blockMode );
+		const targetRange = findDropTargetRange(
+			this.editor,
+			targetViewElement,
+			targetViewRanges,
+			clientX,
+			clientY,
+			blockMode,
+			draggedRange
+		);
 		// The dragging markers must be removed after searching for the target range because sometimes
 		// the target lands on the marker itself.
 		this.removeDropMarker();
@@ -336,7 +363,8 @@ function findDropTargetRange(
 	targetViewRanges: Array<ViewRange> | null,
 	clientX: number,
 	clientY: number,
-	blockMode: boolean
+	blockMode: boolean,
+	draggedRange: LiveRange | null
 ): Range | null {
 	const model = editor.model;
 	const mapper = editor.editing.mapper;
@@ -347,19 +375,26 @@ function findDropTargetRange(
 	while ( modelElement ) {
 		if ( !blockMode ) {
 			if ( model.schema.checkChild( modelElement, '$text' ) ) {
-				const targetViewPosition = targetViewRanges ? targetViewRanges[ 0 ].start : null;
-				const targetModelPosition = targetViewPosition ? mapper.toModelPosition( targetViewPosition ) : null;
-
-				if ( targetModelPosition ) {
-					if ( model.schema.checkChild( targetModelPosition, '$text' ) ) {
-						return model.createRange( targetModelPosition );
-					}
-					else if ( targetViewPosition ) {
+				if ( targetViewRanges ) {
+					const targetViewPosition = targetViewRanges[ 0 ].start;
+					const targetModelPosition = mapper.toModelPosition( targetViewPosition );
+					const canDropOnPosition = !draggedRange || Array
+						.from( draggedRange.getItems() )
+						.every( item => model.schema.checkChild( targetModelPosition, item as Node ) );
+
+					if ( canDropOnPosition ) {
+						if ( model.schema.checkChild( targetModelPosition, '$text' ) ) {
+							return model.createRange( targetModelPosition );
+						}
+						else if ( targetViewPosition ) {
 						// This is the case of dropping inside a span wrapper of an inline image.
-						return findDropTargetRangeForElement( editor,
-							getClosestMappedModelElement( editor, targetViewPosition.parent as ViewElement ),
-							clientX, clientY
-						);
+							return findDropTargetRangeForElement(
+								editor,
+								getClosestMappedModelElement( editor, targetViewPosition.parent as ViewElement ),
+								clientX,
+								clientY
+							);
+						}
 					}
 				}
 			}

diff --git a/packages/ckeditor5-clipboard/tests/dragdrop.js b/packages/ckeditor5-clipboard/tests/dragdrop.js
@@ -479,13 +479,13 @@ describe( 'Drag and Drop', () => {
 
 			dataTransferMock = createDataTransfer( { 'text/html': 'abc' } );
 
-			let targetPosition = model.createPositionAt( root.getChild( 0 ), 2 );
+			let targetPosition = model.createPositionAt( root.getChild( 0 ), 3 );
 			fireDragging( dataTransferMock, targetPosition );
 			clock.tick( 100 );
 
 			expectDraggingMarker( targetPosition );
 			expect( getViewData( view, { renderUIElements: true } ) ).to.equal(
-				'<p>{fo<span class="ck ck-clipboard-drop-target-position">\u2060<span></span>\u2060</span>o}bar</p>'
+				'<p>{foo}<span class="ck ck-clipboard-drop-target-position">\u2060<span></span>\u2060</span>bar</p>'
 			);
 
 			// Dropping.