8
0
Просмотр исходного кода

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.
Piotrek Koszuliński 6 лет назад
Родитель
Сommit
def8c28d3e

+ 11 - 11
packages/ckeditor5-restricted-editing/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;

+ 87 - 0
packages/ckeditor5-restricted-editing/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;
+}

+ 1 - 0
packages/ckeditor5-restricted-editing/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>

+ 255 - 5
packages/ckeditor5-restricted-editing/tests/restrictededitingmodeediting.js

@@ -19,7 +19,7 @@ import RestrictedEditingModeEditing from './../src/restrictededitingmodeediting'
 import RestrictedEditingModeNavigationCommand from '../src/restrictededitingmodenavigationcommand';
 
 describe( 'RestrictedEditingModeEditing', () => {
-	let editor;
+	let editor, model;
 
 	testUtils.createSinonSandbox();
 
@@ -53,8 +53,6 @@ describe( 'RestrictedEditingModeEditing', () => {
 	} );
 
 	describe( 'conversion', () => {
-		let model;
-
 		beforeEach( async () => {
 			editor = await VirtualTestEditor.create( { plugins: [ Paragraph, RestrictedEditingModeEditing ] } );
 			model = editor.model;
@@ -164,8 +162,6 @@ describe( 'RestrictedEditingModeEditing', () => {
 	} );
 
 	describe( 'editing behavior', () => {
-		let model;
-
 		beforeEach( async () => {
 			editor = await VirtualTestEditor.create( { plugins: [ Paragraph, Typing, RestrictedEditingModeEditing ] } );
 			model = editor.model;
@@ -319,6 +315,64 @@ describe( 'RestrictedEditingModeEditing', () => {
 			expect( markerRange.isEqual( expectedRange ) ).to.be.true;
 		} );
 
+		it( 'should retain marker on non-typing change at the marker boundary (start)', () => {
+			setModelData( model, '<paragraph>foo bar[] baz</paragraph>' );
+			const firstParagraph = model.document.getRoot().getChild( 0 );
+			addExceptionMarker( 4, 7, firstParagraph );
+
+			model.change( writer => {
+				editor.execute( 'delete', {
+					selection: writer.createSelection( writer.createRange(
+						writer.createPositionAt( firstParagraph, 4 ),
+						writer.createPositionAt( firstParagraph, 6 )
+					) )
+				} );
+				editor.execute( 'input', {
+					text: 'XX',
+					range: writer.createRange( writer.createPositionAt( firstParagraph, 4 ) )
+				} );
+			} );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>foo XX[]r baz</paragraph>' );
+
+			const markerRange = editor.model.markers.get( 'restrictedEditingException:1' ).getRange();
+			const expectedRange = model.createRange(
+				model.createPositionAt( firstParagraph, 4 ),
+				model.createPositionAt( firstParagraph, 7 )
+			);
+
+			expect( markerRange.isEqual( expectedRange ) ).to.be.true;
+		} );
+
+		it( 'should retain marker on non-typing change at marker boundary (end)', () => {
+			setModelData( model, '<paragraph>foo bar[] baz</paragraph>' );
+			const firstParagraph = model.document.getRoot().getChild( 0 );
+			addExceptionMarker( 4, 7, firstParagraph );
+
+			model.change( writer => {
+				editor.execute( 'delete', {
+					selection: writer.createSelection( writer.createRange(
+						writer.createPositionAt( firstParagraph, 5 ),
+						writer.createPositionAt( firstParagraph, 7 )
+					) )
+				} );
+				editor.execute( 'input', {
+					text: 'XX',
+					range: writer.createRange( writer.createPositionAt( firstParagraph, 5 ) )
+				} );
+			} );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>foo bXX[] baz</paragraph>' );
+
+			const markerRange = editor.model.markers.get( 'restrictedEditingException:1' ).getRange();
+			const expectedRange = model.createRange(
+				model.createPositionAt( firstParagraph, 4 ),
+				model.createPositionAt( firstParagraph, 7 )
+			);
+
+			expect( markerRange.isEqual( expectedRange ) ).to.be.true;
+		} );
+
 		it( 'should not move collapsed marker to $graveyard', () => {
 			setModelData( model, '<paragraph>foo b[]ar baz</paragraph>' );
 			const firstParagraph = model.document.getRoot().getChild( 0 );
@@ -348,6 +402,187 @@ describe( 'RestrictedEditingModeEditing', () => {
 		} );
 	} );
 
+	describe( 'enforcing restrictions on deleteContent', () => {
+		beforeEach( async () => {
+			editor = await VirtualTestEditor.create( { plugins: [ Paragraph, Typing, RestrictedEditingModeEditing ] } );
+			model = editor.model;
+		} );
+
+		afterEach( async () => {
+			await editor.destroy();
+		} );
+
+		it( 'should not allow to delete content outside restricted area', () => {
+			setModelData( model, '<paragraph>[]foo bar baz</paragraph>' );
+			const firstParagraph = model.document.getRoot().getChild( 0 );
+
+			addExceptionMarker( 3, 9, firstParagraph );
+
+			model.change( writer => {
+				writer.setSelection( firstParagraph, 2 );
+			} );
+
+			model.deleteContent( model.document.selection );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>fo[]o bar baz</paragraph>' );
+		} );
+
+		it( 'should trim deleted content to a exception marker (focus in marker)', () => {
+			setModelData( model, '<paragraph>[]foofoo bar baz</paragraph>' );
+			const firstParagraph = model.document.getRoot().getChild( 0 );
+
+			addExceptionMarker( 3, 9, firstParagraph );
+
+			model.change( writer => {
+				const selection = writer.createSelection( writer.createRange(
+					writer.createPositionAt( firstParagraph, 0 ),
+					writer.createPositionAt( firstParagraph, 6 )
+				) );
+				model.deleteContent( selection );
+			} );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>[]foo bar baz</paragraph>' );
+		} );
+
+		it( 'should trim deleted content to a exception marker (anchor in marker)', () => {
+			setModelData( model, '<paragraph>[]foo bar baz</paragraph>' );
+			const firstParagraph = model.document.getRoot().getChild( 0 );
+
+			addExceptionMarker( 4, 7, firstParagraph );
+
+			model.change( writer => {
+				const selection = writer.createSelection( writer.createRange(
+					writer.createPositionAt( firstParagraph, 5 ),
+					writer.createPositionAt( firstParagraph, 8 )
+				) );
+				model.deleteContent( selection );
+			} );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>[]foo b baz</paragraph>' );
+		} );
+
+		it( 'should trim deleted content to a exception marker and alter the selection argument (delete command integration)', () => {
+			setModelData( model, '<paragraph>[]foofoo bar baz</paragraph>' );
+			const firstParagraph = model.document.getRoot().getChild( 0 );
+
+			addExceptionMarker( 3, 9, firstParagraph );
+
+			model.change( writer => {
+				writer.setSelection( firstParagraph, 6 );
+			} );
+			editor.execute( 'delete', { unit: 'word' } );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>foo[] bar baz</paragraph>' );
+		} );
+
+		it( 'should work with document selection', () => {
+			setModelData( model, '<paragraph>f[oo bar] baz</paragraph>' );
+			const firstParagraph = model.document.getRoot().getChild( 0 );
+
+			addExceptionMarker( 2, 'end', firstParagraph );
+
+			model.change( () => {
+				model.deleteContent( model.document.selection );
+			} );
+
+			assertEqualMarkup( getModelData( model, { withoutSelection: true } ), '<paragraph>fo baz</paragraph>' );
+		} );
+	} );
+
+	describe( 'enforcing restrictions on input command', () => {
+		let firstParagraph;
+
+		beforeEach( async () => {
+			editor = await VirtualTestEditor.create( { plugins: [ Paragraph, Typing, RestrictedEditingModeEditing ] } );
+			model = editor.model;
+
+			setModelData( model, '<paragraph>[]foo bar baz</paragraph>' );
+
+			firstParagraph = model.document.getRoot().getChild( 0 );
+		} );
+
+		afterEach( async () => {
+			await editor.destroy();
+		} );
+
+		it( 'should prevent changing text before exception marker', () => {
+			addExceptionMarker( 4, 7, firstParagraph );
+
+			model.change( writer => {
+				writer.setSelection( firstParagraph, 5 );
+			} );
+
+			// Simulate native spell-check action.
+			editor.execute( 'input', {
+				text: 'xxxxxxx',
+				range: model.createRange(
+					model.createPositionAt( firstParagraph, 0 ),
+					model.createPositionAt( firstParagraph, 7 )
+				)
+			} );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>foo b[]ar baz</paragraph>' );
+		} );
+
+		it( 'should prevent changing text before exception marker', () => {
+			addExceptionMarker( 4, 7, firstParagraph );
+
+			model.change( writer => {
+				writer.setSelection( firstParagraph, 5 );
+			} );
+
+			// Simulate native spell-check action.
+			editor.execute( 'input', {
+				text: 'xxxxxxx',
+				range: model.createRange(
+					model.createPositionAt( firstParagraph, 4 ),
+					model.createPositionAt( firstParagraph, 9 )
+				)
+			} );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>foo b[]ar baz</paragraph>' );
+		} );
+
+		it( 'should prevent changing text before (change crossing different markers)', () => {
+			addExceptionMarker( 0, 4, firstParagraph );
+			addExceptionMarker( 7, 9, firstParagraph, 2 );
+
+			model.change( writer => {
+				writer.setSelection( firstParagraph, 2 );
+			} );
+
+			// Simulate native spell-check action.
+			editor.execute( 'input', {
+				text: 'xxxxxxx',
+				range: model.createRange(
+					model.createPositionAt( firstParagraph, 2 ),
+					model.createPositionAt( firstParagraph, 8 )
+				)
+			} );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>fo[]o bar baz</paragraph>' );
+		} );
+
+		it( 'should allow changing text inside single marker', () => {
+			addExceptionMarker( 0, 9, firstParagraph );
+
+			model.change( writer => {
+				writer.setSelection( firstParagraph, 2 );
+			} );
+
+			// Simulate native spell-check action.
+			editor.execute( 'input', {
+				text: 'xxxxxxx',
+				range: model.createRange(
+					model.createPositionAt( firstParagraph, 2 ),
+					model.createPositionAt( firstParagraph, 8 )
+				)
+			} );
+
+			assertEqualMarkup( getModelData( model ), '<paragraph>foxxxxxxx[]baz</paragraph>' );
+		} );
+	} );
+
 	describe( 'clipboard', () => {
 		let model, viewDoc;
 
@@ -873,4 +1108,19 @@ describe( 'RestrictedEditingModeEditing', () => {
 			sinon.assert.calledOnce( domEvtDataStub.stopPropagation );
 		} );
 	} );
+
+	// Helper method that creates an exception marker inside given parent.
+	// Marker range is set to given position offsets (start, end).
+	function addExceptionMarker( startOffset, endOffset = startOffset, parent, id = 1 ) {
+		model.change( writer => {
+			writer.addMarker( `restrictedEditingException:${ id }`, {
+				range: writer.createRange(
+					writer.createPositionAt( parent, startOffset ),
+					writer.createPositionAt( parent, endOffset )
+				),
+				usingOperation: true,
+				affectsData: true
+			} );
+		} );
+	}
 } );