Merge pull request #7 from ckeditor/i/5840-safe

Fix: Restricted editing boundaries should not be crossed by delete content and input command. Closes ckeditor/ckeditor5#5840.

src/restrictededitingmode/converters.js

@@ -103,9 +103,9 @@ export function extendMarkerOnTypingPostFixer( editor ) {
 		let changeApplied = false;
 
 		for ( const change of editor.model.document.differ.getChanges() ) {
-			if ( change.type == 'insert' && change.name == '$text' && change.length === 1 ) {
-				changeApplied = _tryExtendMarkerStart( editor, change.position, writer ) || changeApplied;
-				changeApplied = _tryExtendMarkedEnd( editor, change.position, writer ) || changeApplied;
+			if ( change.type == 'insert' && change.name == '$text' ) {
+				changeApplied = _tryExtendMarkerStart( editor, change.position, change.length, writer ) || changeApplied;
+				changeApplied = _tryExtendMarkedEnd( editor, change.position, change.length, writer ) || changeApplied;
 			}
 		}
 
@@ -159,13 +159,13 @@ export function upcastHighlightToMarker( config ) {
 	} );
 }
 
-// Extend marker if typing detected on marker's start position.
-function _tryExtendMarkerStart( editor, position, writer ) {
-	const markerAtStart = getMarkerAtPosition( editor, position.getShiftedBy( 1 ) );
+// Extend marker if change detected on marker's start position.
+function _tryExtendMarkerStart( editor, position, length, writer ) {
+	const markerAtStart = getMarkerAtPosition( editor, position.getShiftedBy( length ) );
 
-	if ( markerAtStart && markerAtStart.getStart().isEqual( position.getShiftedBy( 1 ) ) ) {
+	if ( markerAtStart && markerAtStart.getStart().isEqual( position.getShiftedBy( length ) ) ) {
 		writer.updateMarker( markerAtStart, {
-			range: writer.createRange( markerAtStart.getStart().getShiftedBy( -1 ), markerAtStart.getEnd() )
+			range: writer.createRange( markerAtStart.getStart().getShiftedBy( -length ), markerAtStart.getEnd() )
 		} );
 
 		return true;
@@ -174,13 +174,13 @@ function _tryExtendMarkerStart( editor, position, writer ) {
 	return false;
 }
 
-// Extend marker if typing detected on marker's end position.
-function _tryExtendMarkedEnd( editor, position, writer ) {
+// Extend marker if change detected on marker's end position.
+function _tryExtendMarkedEnd( editor, position, length, writer ) {
 	const markerAtEnd = getMarkerAtPosition( editor, position );
 
 	if ( markerAtEnd && markerAtEnd.getEnd().isEqual( position ) ) {
 		writer.updateMarker( markerAtEnd, {
-			range: writer.createRange( markerAtEnd.getStart(), markerAtEnd.getEnd().getShiftedBy( 1 ) )
+			range: writer.createRange( markerAtEnd.getStart(), markerAtEnd.getEnd().getShiftedBy( length ) )
 		} );
 
 		return true;

src/restrictededitingmodeediting.js

@@ -76,6 +76,7 @@ export default class RestrictedEditingModeEditing extends Plugin {
 
 		this._setupConversion();
 		this._setupCommandsToggling();
+		this._setupRestrictions();
 
 		// Commands & keystrokes that allow navigation in the content.
 		editor.commands.add( 'goToPreviousRestrictedEditingException', new RestrictedEditingNavigationCommand( editor, 'backward' ) );
@@ -165,6 +166,25 @@ export default class RestrictedEditingModeEditing extends Plugin {
 		setupExceptionHighlighting( editor );
 	}
 
+	/**
+	 * Setups additional editing restrictions beyond command toggling.
+	 *
+	 * @private
+	 */
+	_setupRestrictions() {
+		const editor = this.editor;
+
+		this.listenTo( editor.model, 'deleteContent', restrictDeleteContent( editor ), { priority: 'high' } );
+
+		const inputCommand = this.editor.commands.get( 'input' );
+
+		// The restricted editing might be configured without input support - ie allow only bolding or removing text.
+		// This check is bit synthetic since only tests are used this way.
+		if ( inputCommand ) {
+			this.listenTo( inputCommand, 'execute', disallowInputExecForWrongRange( editor ), { priority: 'high' } );
+		}
+	}
+
 	/**
 	 * Setups the commands toggling - enables or disables commands based on user selection.
 	 *
@@ -284,3 +304,70 @@ function filterDeleteCommandsOnMarkerBoundaries( selection, markerRange ) {
 		return true;
 	};
 }
+
+// Ensures that model.deleteContent() does not delete outside exception markers ranges.
+//
+// The enforced restrictions are:
+// - only execute deleteContent() inside exception markers
+// - restrict passed selection to exception marker
+function restrictDeleteContent( editor ) {
+	return ( evt, args ) => {
+		const [ selection ] = args;
+
+		const marker = getMarkerAtPosition( editor, selection.focus ) || getMarkerAtPosition( editor, selection.anchor );
+
+		// Stop method execution if marker was not found at selection focus.
+		if ( !marker ) {
+			evt.stop();
+
+			return;
+		}
+
+		// Collapsed selection inside exception marker does not require fixing.
+		if ( selection.isCollapsed ) {
+			return;
+		}
+
+		// Shrink the selection to the range inside exception marker.
+		const allowedToDelete = marker.getRange().getIntersection( selection.getFirstRange() );
+
+		// Some features uses selection passed to model.deleteContent() to set the selection afterwards. For this we need to properly modify
+		// either the document selection using change block...
+		if ( selection.is( 'documentSelection' ) ) {
+			editor.model.change( writer => {
+				writer.setSelection( allowedToDelete );
+			} );
+		}
+		// ... or by modifying passed selection instance directly.
+		else {
+			selection.setTo( allowedToDelete );
+		}
+	};
+}
+
+// Ensures that input command is executed with a range that is inside exception marker.
+//
+// This restriction is due to fact that using native spell check changes text outside exception marker.
+function disallowInputExecForWrongRange( editor ) {
+	return ( evt, args ) => {
+		const [ options ] = args;
+		const { range } = options;
+
+		// Only check "input" command executed with a range value.
+		// Selection might be set in exception marker but passed range might point elsewhere.
+		if ( !range ) {
+			return;
+		}
+
+		if ( !isRangeInsideSingleMarker( editor, range ) ) {
+			evt.stop();
+		}
+	};
+}
+
+function isRangeInsideSingleMarker( editor, range ) {
+	const markerAtStart = getMarkerAtPosition( editor, range.start );
+	const markerAtEnd = getMarkerAtPosition( editor, range.end );
+
+	return markerAtStart && markerAtEnd && markerAtEnd === markerAtStart;
+}

tests/manual/restrictedediting.html

@@ -7,6 +7,7 @@
 <div id="editor">
 	<h2>Heading 1</h2>
 	<p>Paragraph <span class="restricted-editing-exception">it is editable</span></p>
+	<p>Exception on part of a word: <a href="ckeditor.com">coompi</a><span class="restricted-editing-exception"><a href="ckeditor.com">ter</a> (fix spelling in Firefox).</span></p>
 	<p><strong>Bold</strong> <i>Italic</i> <a href="foo">Link</a></p>
 	<ul>
 		<li>UL List item 1</li>