From 8c2ec6a6cd17f0ad4a923c8ccc07445cb03079ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 2 Dec 2019 19:16:08 +0100 Subject: [PATCH 01/13] Modify selection before removing content. --- src/restrictededitingmodeediting.js | 18 +++++++++++++ tests/restrictededitingmodeediting.js | 38 ++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index 3b6f3c1c..d1471cec 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -162,6 +162,24 @@ export default class RestrictedEditingModeEditing extends Plugin { doc.registerPostFixer( extendMarkerOnTypingPostFixer( editor ) ); doc.registerPostFixer( resurrectCollapsedMarkerPostFixer( editor ) ); + editor.model.on( 'deleteContent', ( evt, args ) => { + // console.log( 'bofore:deleteContent', evt.stop() ); + const [ selection, ] = args; + + const marker = getMarkerAtPosition( editor, selection.focus ) || getMarkerAtPosition( editor, selection.anchor ); + + if ( !marker ) { + evt.stop(); + return; + } + + const intersection = marker.getRange().getIntersection( selection.getFirstRange() ); + + const allowedToDelete = model.createSelection( intersection ); + + args.splice( 0, 1, allowedToDelete ); + }, { priority: 'high' } ); + setupExceptionHighlighting( editor ); } diff --git a/tests/restrictededitingmodeediting.js b/tests/restrictededitingmodeediting.js index 841742c2..7bcb09eb 100644 --- a/tests/restrictededitingmodeediting.js +++ b/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(); @@ -348,6 +348,32 @@ describe( 'RestrictedEditingModeEditing', () => { } ); } ); + describe.only( 'post-fixer', () => { + beforeEach( async () => { + editor = await VirtualTestEditor.create( { plugins: [ Paragraph, Typing, RestrictedEditingModeEditing ] } ); + model = editor.model; + } ); + + afterEach( () => { + return editor.destroy(); + } ); + + it( 'should not allow to change text outside restricted area', () => { + setModelData( model, '[]foofoo bar baz' ); + 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 ), 'foo[] bar baz' ); + } ); + } ); + describe( 'clipboard', () => { let model, viewDoc; @@ -873,4 +899,14 @@ describe( 'RestrictedEditingModeEditing', () => { sinon.assert.calledOnce( domEvtDataStub.stopPropagation ); } ); } ); + + function addExceptionMarker( start, end = start, parent, id = 1 ) { + model.change( writer => { + writer.addMarker( `restrictedEditingException:${ id }`, { + range: writer.createRange( writer.createPositionAt( parent, start ), writer.createPositionAt( parent, end ) ), + usingOperation: true, + affectsData: true + } ); + } ); + } } ); From a0e81b07d1603d4fe8e1214725b393da7dd5f098 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 3 Dec 2019 17:41:44 +0100 Subject: [PATCH 02/13] Do not alter passed arguments when fixing selection passed to model.deleteContent(). --- src/restrictededitingmodeediting.js | 48 +++++++++++------- tests/restrictededitingmodeediting.js | 71 ++++++++++++++++++++++----- 2 files changed, 91 insertions(+), 28 deletions(-) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index d1471cec..d08c2668 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -162,23 +162,7 @@ export default class RestrictedEditingModeEditing extends Plugin { doc.registerPostFixer( extendMarkerOnTypingPostFixer( editor ) ); doc.registerPostFixer( resurrectCollapsedMarkerPostFixer( editor ) ); - editor.model.on( 'deleteContent', ( evt, args ) => { - // console.log( 'bofore:deleteContent', evt.stop() ); - const [ selection, ] = args; - - const marker = getMarkerAtPosition( editor, selection.focus ) || getMarkerAtPosition( editor, selection.anchor ); - - if ( !marker ) { - evt.stop(); - return; - } - - const intersection = marker.getRange().getIntersection( selection.getFirstRange() ); - - const allowedToDelete = model.createSelection( intersection ); - - args.splice( 0, 1, allowedToDelete ); - }, { priority: 'high' } ); + this.listenTo( model, 'deleteContent', restrictDeleteContent( editor ), { priority: 'high' } ); setupExceptionHighlighting( editor ); } @@ -302,3 +286,33 @@ 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; + } + + const allowedToDelete = marker.getRange().getIntersection( selection.getFirstRange() ); + + // Shrink the selection to the range inside exception marker. + selection.setTo( allowedToDelete ); + }; +} diff --git a/tests/restrictededitingmodeediting.js b/tests/restrictededitingmodeediting.js index 7bcb09eb..54b51d03 100644 --- a/tests/restrictededitingmodeediting.js +++ b/tests/restrictededitingmodeediting.js @@ -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; @@ -348,26 +344,74 @@ describe( 'RestrictedEditingModeEditing', () => { } ); } ); - describe.only( 'post-fixer', () => { + describe( 'post-fixer', () => { beforeEach( async () => { editor = await VirtualTestEditor.create( { plugins: [ Paragraph, Typing, RestrictedEditingModeEditing ] } ); model = editor.model; } ); - afterEach( () => { - return editor.destroy(); + afterEach( async () => { + await editor.destroy(); } ); - it( 'should not allow to change text outside restricted area', () => { + it( 'should not allow to delete content outside restricted area', () => { + setModelData( model, '[]foo bar baz' ); + 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 ), 'fo[]o bar baz' ); + } ); + + it( 'should trim deleted content to a exception marker (focus in marker)', () => { setModelData( model, '[]foofoo bar baz' ); const firstParagraph = model.document.getRoot().getChild( 0 ); addExceptionMarker( 3, 9, firstParagraph ); model.change( writer => { - writer.setSelection( firstParagraph, 6 ); + const selection = writer.createSelection( writer.createRange( + writer.createPositionAt( firstParagraph, 0 ), + writer.createPositionAt( firstParagraph, 6 ) + ) ); + model.deleteContent( selection ); } ); + assertEqualMarkup( getModelData( model ), '[]foo bar baz' ); + } ); + + it( 'should trim deleted content to a exception marker (anchor in marker)', () => { + setModelData( model, '[]foo bar baz' ); + 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 ), '[]foo b baz' ); + } ); + + it( 'should trim deleted content to a exception marker and alter the selection argument (delete command integration)', () => { + setModelData( model, '[]foofoo bar baz' ); + 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 ), 'foo[] bar baz' ); @@ -900,10 +944,15 @@ describe( 'RestrictedEditingModeEditing', () => { } ); } ); - function addExceptionMarker( start, end = start, parent, id = 1 ) { + // 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, start ), writer.createPositionAt( parent, end ) ), + range: writer.createRange( + writer.createPositionAt( parent, startOffset ), + writer.createPositionAt( parent, endOffset ) + ), usingOperation: true, affectsData: true } ); From cab7b583f2266ab1565f3231334e1ad8893a1652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 3 Dec 2019 18:09:35 +0100 Subject: [PATCH 03/13] Change describe group name for enforcing restrictions on deleteContent. --- tests/restrictededitingmodeediting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/restrictededitingmodeediting.js b/tests/restrictededitingmodeediting.js index 54b51d03..8577dd1d 100644 --- a/tests/restrictededitingmodeediting.js +++ b/tests/restrictededitingmodeediting.js @@ -344,7 +344,7 @@ describe( 'RestrictedEditingModeEditing', () => { } ); } ); - describe( 'post-fixer', () => { + describe( 'enforcing restrictions on deleteContent', () => { beforeEach( async () => { editor = await VirtualTestEditor.create( { plugins: [ Paragraph, Typing, RestrictedEditingModeEditing ] } ); model = editor.model; From 9980f6719709fd5a105e2f6871dcac2425a6105e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 3 Dec 2019 18:26:32 +0100 Subject: [PATCH 04/13] Add enforcing restrictions on remove attribute operation. --- src/restrictededitingmodeediting.js | 25 +++++++++++ tests/restrictededitingmodeediting.js | 61 +++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index d08c2668..0da083c7 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -163,6 +163,7 @@ export default class RestrictedEditingModeEditing extends Plugin { doc.registerPostFixer( resurrectCollapsedMarkerPostFixer( editor ) ); this.listenTo( model, 'deleteContent', restrictDeleteContent( editor ), { priority: 'high' } ); + this.listenTo( model, 'applyOperation', restrictAttributeOperation( editor ), { priority: 'high' } ); setupExceptionHighlighting( editor ); } @@ -316,3 +317,27 @@ function restrictDeleteContent( editor ) { selection.setTo( allowedToDelete ); }; } + +// Ensures that remove attribute operation is executed on proper range. +// +// The restriction is enforced by trimming range of an AttributeOperation: +function restrictAttributeOperation( editor ) { + return ( evt, args ) => { + const [ operation ] = args; + + if ( operation.type != 'removeAttribute' ) { + return; + } + + const marker = getMarkerAtPosition( editor, operation.range.start ) || getMarkerAtPosition( editor, operation.range.end ); + + // Stop method execution if marker was not found at selection focus. + if ( !marker ) { + evt.stop(); + + return; + } + + operation.range = operation.range.getIntersection( marker.getRange() ); + }; +} diff --git a/tests/restrictededitingmodeediting.js b/tests/restrictededitingmodeediting.js index 8577dd1d..b38025c0 100644 --- a/tests/restrictededitingmodeediting.js +++ b/tests/restrictededitingmodeediting.js @@ -418,6 +418,67 @@ describe( 'RestrictedEditingModeEditing', () => { } ); } ); + describe( 'enforcing restrictions on remove attribute operation', () => { + beforeEach( async () => { + editor = await VirtualTestEditor.create( { plugins: [ Paragraph, Typing, RestrictedEditingModeEditing ] } ); + model = editor.model; + + editor.model.schema.extend( '$text', { allowAttributes: [ 'link' ] } ); + } ); + + afterEach( async () => { + await editor.destroy(); + } ); + + it( 'should not allow to delete content outside restricted area', () => { + setModelData( model, '<$text link="true">[]foo bar baz' ); + const firstParagraph = model.document.getRoot().getChild( 0 ); + addExceptionMarker( 4, 7, firstParagraph ); + + model.change( writer => { + writer.removeAttribute( 'link', writer.createRange( + writer.createPositionAt( firstParagraph, 0 ), + writer.createPositionAt( firstParagraph, 1 ) + ) ); + } ); + + assertEqualMarkup( getModelData( model ), '<$text link="true">[]foo bar baz' ); + } ); + + it( 'should trim deleted content to a exception marker (end in marker)', () => { + setModelData( model, '<$text link="true">[]foo bar baz' ); + const firstParagraph = model.document.getRoot().getChild( 0 ); + addExceptionMarker( 4, 7, firstParagraph ); + + model.change( writer => { + writer.removeAttribute( 'link', writer.createRange( + writer.createPositionAt( firstParagraph, 0 ), + writer.createPositionAt( firstParagraph, 7 ) + ) ); + } ); + + assertEqualMarkup( getModelData( model ), '<$text link="true">[]foo bar baz' ); + } ); + + it( 'should trim deleted content to a exception marker (start in marker)', () => { + setModelData( model, '<$text link="true">[]foo bar baz' ); + const firstParagraph = model.document.getRoot().getChild( 0 ); + addExceptionMarker( 4, 7, firstParagraph ); + + model.change( writer => { + writer.removeAttribute( 'link', writer.createRange( + writer.createPositionAt( firstParagraph, 5 ), + writer.createPositionAt( firstParagraph, 8 ) + ) ); + } ); + + assertEqualMarkup( + getModelData( model ), + '<$text link="true">[]foo bar<$text link="true"> baz' + ); + } ); + } ); + describe( 'clipboard', () => { let model, viewDoc; From 5f980828d9d517e6a72658e63aaa264e43a5362f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 3 Dec 2019 18:40:20 +0100 Subject: [PATCH 05/13] Fix internal docs. --- src/restrictededitingmodeediting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index 0da083c7..ec0c20b7 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -320,7 +320,7 @@ function restrictDeleteContent( editor ) { // Ensures that remove attribute operation is executed on proper range. // -// The restriction is enforced by trimming range of an AttributeOperation: +// The restriction is enforced by trimming range of an AttributeOperation. function restrictAttributeOperation( editor ) { return ( evt, args ) => { const [ operation ] = args; From 4aafc3c38c42b6b8c55c92e84832c97a1d5ee09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 3 Dec 2019 19:43:47 +0100 Subject: [PATCH 06/13] Enforce input command to the ranges inside exception marker. --- src/restrictededitingmodeediting.js | 35 ++++++++++ tests/manual/restrictedediting.html | 1 + tests/restrictededitingmodeediting.js | 94 +++++++++++++++++++++++++++ 3 files changed, 130 insertions(+) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index ec0c20b7..2341f4b9 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -165,6 +165,14 @@ export default class RestrictedEditingModeEditing extends Plugin { this.listenTo( model, 'deleteContent', restrictDeleteContent( editor ), { priority: 'high' } ); this.listenTo( model, 'applyOperation', restrictAttributeOperation( 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', restrictInputRangeOption( editor ), { priority: 'high' } ); + } + setupExceptionHighlighting( editor ); } @@ -341,3 +349,30 @@ function restrictAttributeOperation( editor ) { operation.range = operation.range.getIntersection( marker.getRange() ); }; } + +// 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 restrictInputRangeOption( 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; +} diff --git a/tests/manual/restrictedediting.html b/tests/manual/restrictedediting.html index 9186494f..877f689a 100644 --- a/tests/manual/restrictedediting.html +++ b/tests/manual/restrictedediting.html @@ -7,6 +7,7 @@

Heading 1

Paragraph it is editable

+

Exception on part of a word: coompiter (fix spelling in Firefox).

Bold Italic Link

  • UL List item 1
  • diff --git a/tests/restrictededitingmodeediting.js b/tests/restrictededitingmodeediting.js index b38025c0..e30468f2 100644 --- a/tests/restrictededitingmodeediting.js +++ b/tests/restrictededitingmodeediting.js @@ -479,6 +479,100 @@ describe( 'RestrictedEditingModeEditing', () => { } ); } ); + describe( 'enforcing restrictions on input command', () => { + let firstParagraph; + + beforeEach( async () => { + editor = await VirtualTestEditor.create( { plugins: [ Paragraph, Typing, RestrictedEditingModeEditing ] } ); + model = editor.model; + + setModelData( model, '[]foo bar baz' ); + + 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 ), 'foo b[]ar baz' ); + } ); + + 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 ), 'foo b[]ar baz' ); + } ); + + 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 ), 'fo[]o bar baz' ); + } ); + + 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 ), 'foxxxxxxx[]baz' ); + } ); + } ); + describe( 'clipboard', () => { let model, viewDoc; From 8d70bad983c0802ef5d8f5f431c6f1b7fd554820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 4 Dec 2019 10:59:57 +0100 Subject: [PATCH 07/13] Move restriction enforcing related commands to private method. --- src/restrictededitingmodeediting.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index 2341f4b9..ebbaef91 100644 --- a/src/restrictededitingmodeediting.js +++ b/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' ) ); @@ -162,8 +163,14 @@ export default class RestrictedEditingModeEditing extends Plugin { doc.registerPostFixer( extendMarkerOnTypingPostFixer( editor ) ); doc.registerPostFixer( resurrectCollapsedMarkerPostFixer( editor ) ); - this.listenTo( model, 'deleteContent', restrictDeleteContent( editor ), { priority: 'high' } ); - this.listenTo( model, 'applyOperation', restrictAttributeOperation( editor ), { priority: 'high' } ); + setupExceptionHighlighting( editor ); + } + + _setupRestrictions() { + const editor = this.editor; + + this.listenTo( editor.model, 'deleteContent', restrictDeleteContent( editor ), { priority: 'high' } ); + this.listenTo( editor.model, 'applyOperation', restrictAttributeOperation( editor ), { priority: 'high' } ); const inputCommand = this.editor.commands.get( 'input' ); @@ -172,8 +179,6 @@ export default class RestrictedEditingModeEditing extends Plugin { if ( inputCommand ) { this.listenTo( inputCommand, 'execute', restrictInputRangeOption( editor ), { priority: 'high' } ); } - - setupExceptionHighlighting( editor ); } /** From c171bca0da8a571680ebe486769ffb7012c17592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 4 Dec 2019 11:06:49 +0100 Subject: [PATCH 08/13] Rename restrictInputRangeOption() to disallowInputExecForWrongRange(). --- src/restrictededitingmodeediting.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index ebbaef91..9dea87a4 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -166,6 +166,11 @@ export default class RestrictedEditingModeEditing extends Plugin { setupExceptionHighlighting( editor ); } + /** + * Setups additional editing restrictions beyond command toggling. + * + * @private + */ _setupRestrictions() { const editor = this.editor; @@ -177,7 +182,7 @@ export default class RestrictedEditingModeEditing extends Plugin { // 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', restrictInputRangeOption( editor ), { priority: 'high' } ); + this.listenTo( inputCommand, 'execute', disallowInputExecForWrongRange( editor ), { priority: 'high' } ); } } @@ -358,7 +363,7 @@ function restrictAttributeOperation( editor ) { // 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 restrictInputRangeOption( editor ) { +function disallowInputExecForWrongRange( editor ) { return ( evt, args ) => { const [ options ] = args; const { range } = options; From 37e717ce43e95e55efe6c5e55b47efe46d3df2a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 17 Dec 2019 12:01:32 +0100 Subject: [PATCH 09/13] Remove attribute restriction enforcing. --- src/restrictededitingmodeediting.js | 25 ----------- tests/restrictededitingmodeediting.js | 61 --------------------------- 2 files changed, 86 deletions(-) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index 38f9ebe6..48824ba5 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -175,7 +175,6 @@ export default class RestrictedEditingModeEditing extends Plugin { const editor = this.editor; this.listenTo( editor.model, 'deleteContent', restrictDeleteContent( editor ), { priority: 'high' } ); - this.listenTo( editor.model, 'applyOperation', restrictAttributeOperation( editor ), { priority: 'high' } ); const inputCommand = this.editor.commands.get( 'input' ); @@ -336,30 +335,6 @@ function restrictDeleteContent( editor ) { }; } -// Ensures that remove attribute operation is executed on proper range. -// -// The restriction is enforced by trimming range of an AttributeOperation. -function restrictAttributeOperation( editor ) { - return ( evt, args ) => { - const [ operation ] = args; - - if ( operation.type != 'removeAttribute' ) { - return; - } - - const marker = getMarkerAtPosition( editor, operation.range.start ) || getMarkerAtPosition( editor, operation.range.end ); - - // Stop method execution if marker was not found at selection focus. - if ( !marker ) { - evt.stop(); - - return; - } - - operation.range = operation.range.getIntersection( marker.getRange() ); - }; -} - // 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. diff --git a/tests/restrictededitingmodeediting.js b/tests/restrictededitingmodeediting.js index 6c6ca5da..adc8eeb3 100644 --- a/tests/restrictededitingmodeediting.js +++ b/tests/restrictededitingmodeediting.js @@ -418,67 +418,6 @@ describe( 'RestrictedEditingModeEditing', () => { } ); } ); - describe( 'enforcing restrictions on remove attribute operation', () => { - beforeEach( async () => { - editor = await VirtualTestEditor.create( { plugins: [ Paragraph, Typing, RestrictedEditingModeEditing ] } ); - model = editor.model; - - editor.model.schema.extend( '$text', { allowAttributes: [ 'link' ] } ); - } ); - - afterEach( async () => { - await editor.destroy(); - } ); - - it( 'should not allow to delete content outside restricted area', () => { - setModelData( model, '<$text link="true">[]foo bar baz' ); - const firstParagraph = model.document.getRoot().getChild( 0 ); - addExceptionMarker( 4, 7, firstParagraph ); - - model.change( writer => { - writer.removeAttribute( 'link', writer.createRange( - writer.createPositionAt( firstParagraph, 0 ), - writer.createPositionAt( firstParagraph, 1 ) - ) ); - } ); - - assertEqualMarkup( getModelData( model ), '<$text link="true">[]foo bar baz' ); - } ); - - it( 'should trim deleted content to a exception marker (end in marker)', () => { - setModelData( model, '<$text link="true">[]foo bar baz' ); - const firstParagraph = model.document.getRoot().getChild( 0 ); - addExceptionMarker( 4, 7, firstParagraph ); - - model.change( writer => { - writer.removeAttribute( 'link', writer.createRange( - writer.createPositionAt( firstParagraph, 0 ), - writer.createPositionAt( firstParagraph, 7 ) - ) ); - } ); - - assertEqualMarkup( getModelData( model ), '<$text link="true">[]foo bar baz' ); - } ); - - it( 'should trim deleted content to a exception marker (start in marker)', () => { - setModelData( model, '<$text link="true">[]foo bar baz' ); - const firstParagraph = model.document.getRoot().getChild( 0 ); - addExceptionMarker( 4, 7, firstParagraph ); - - model.change( writer => { - writer.removeAttribute( 'link', writer.createRange( - writer.createPositionAt( firstParagraph, 5 ), - writer.createPositionAt( firstParagraph, 8 ) - ) ); - } ); - - assertEqualMarkup( - getModelData( model ), - '<$text link="true">[]foo bar<$text link="true"> baz' - ); - } ); - } ); - describe( 'enforcing restrictions on input command', () => { let firstParagraph; From 4e57a79aba43c18802a063ab257853eaeab1ab39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 18 Dec 2019 13:39:29 +0100 Subject: [PATCH 10/13] The delete content argument fixer should work with document selection. --- src/restrictededitingmodeediting.js | 15 +++++++++++++-- tests/restrictededitingmodeediting.js | 13 +++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index 48824ba5..c6c6e219 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -328,10 +328,21 @@ function restrictDeleteContent( editor ) { return; } + // Shrink the selection to the range inside exception marker. const allowedToDelete = marker.getRange().getIntersection( selection.getFirstRange() ); - // Shrink the selection to the range inside exception marker. - selection.setTo( allowedToDelete ); + // Because the DeleteCommand uses the same selection to set selection after calling model.deleteContent() it is better to modify + // the argument selection. The only problem is when that when DocumentSelection is passed (or used internally) as it does allows + // to be modified. + // Instead we change the arguments passed to `deleteContent()` method when document selection was passed. + if ( selection.is( 'documentSelection' ) ) { + args.splice( 0, 1, editor.model.createSelection( allowedToDelete ) ); + } + // We need to modify selection passed to deleteContent if it is an instance of selection because DeleteCommand uses passed + // selection to set selection afterwards. Since we modifying this here the selection set after the delete content will be invalid. + else { + selection.setTo( allowedToDelete ); + } }; } diff --git a/tests/restrictededitingmodeediting.js b/tests/restrictededitingmodeediting.js index adc8eeb3..b3629cfd 100644 --- a/tests/restrictededitingmodeediting.js +++ b/tests/restrictededitingmodeediting.js @@ -416,6 +416,19 @@ describe( 'RestrictedEditingModeEditing', () => { assertEqualMarkup( getModelData( model ), 'foo[] bar baz' ); } ); + + it( 'should work with document selection', () => { + setModelData( model, 'foo [bar] baz' ); + const firstParagraph = model.document.getRoot().getChild( 0 ); + + addExceptionMarker( 2, 'end', firstParagraph ); + + model.change( () => { + model.deleteContent( model.document.selection ); + } ); + + assertEqualMarkup( getModelData( model ), 'foo [] baz' ); + } ); } ); describe( 'enforcing restrictions on input command', () => { From cf865aa012a24febaa4d7e1da7de4876b5e3bcd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 18 Dec 2019 16:41:53 +0100 Subject: [PATCH 11/13] Extend exception marker on spell checking change. --- src/restrictededitingmode/converters.js | 22 ++++----- tests/restrictededitingmodeediting.js | 62 ++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/restrictededitingmode/converters.js b/src/restrictededitingmode/converters.js index efa27a10..2c6e5d8b 100644 --- a/src/restrictededitingmode/converters.js +++ b/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; diff --git a/tests/restrictededitingmodeediting.js b/tests/restrictededitingmodeediting.js index b3629cfd..20ba3ba5 100644 --- a/tests/restrictededitingmodeediting.js +++ b/tests/restrictededitingmodeediting.js @@ -315,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, 'foo bar[] baz' ); + 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 ), 'foo XX[]r baz' ); + + 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, 'foo bar[] baz' ); + 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 ), 'foo bXX[] baz' ); + + 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, 'foo b[]ar baz' ); const firstParagraph = model.document.getRoot().getChild( 0 ); @@ -418,7 +476,7 @@ describe( 'RestrictedEditingModeEditing', () => { } ); it( 'should work with document selection', () => { - setModelData( model, 'foo [bar] baz' ); + setModelData( model, 'f[oo bar] baz' ); const firstParagraph = model.document.getRoot().getChild( 0 ); addExceptionMarker( 2, 'end', firstParagraph ); @@ -427,7 +485,7 @@ describe( 'RestrictedEditingModeEditing', () => { model.deleteContent( model.document.selection ); } ); - assertEqualMarkup( getModelData( model ), 'foo [] baz' ); + assertEqualMarkup( getModelData( model, { withoutSelection: true } ), 'fo baz' ); } ); } ); From f5527319bd79af6326e201d3ee6a031715b565fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 19 Dec 2019 11:59:34 +0100 Subject: [PATCH 12/13] Use array assignment instead of splice when changing arguments. --- src/restrictededitingmodeediting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index c6c6e219..4a42b62d 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -336,7 +336,7 @@ function restrictDeleteContent( editor ) { // to be modified. // Instead we change the arguments passed to `deleteContent()` method when document selection was passed. if ( selection.is( 'documentSelection' ) ) { - args.splice( 0, 1, editor.model.createSelection( allowedToDelete ) ); + args[ 0 ] = editor.model.createSelection( allowedToDelete ); } // We need to modify selection passed to deleteContent if it is an instance of selection because DeleteCommand uses passed // selection to set selection afterwards. Since we modifying this here the selection set after the delete content will be invalid. From 8272933b3fbbfb6789f4846c9f6367a49089ca4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 19 Dec 2019 14:48:11 +0100 Subject: [PATCH 13/13] Modify document selection in change block in deleteContent decorator method. --- src/restrictededitingmodeediting.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/restrictededitingmodeediting.js b/src/restrictededitingmodeediting.js index 4a42b62d..ebdb9133 100644 --- a/src/restrictededitingmodeediting.js +++ b/src/restrictededitingmodeediting.js @@ -331,15 +331,14 @@ function restrictDeleteContent( editor ) { // Shrink the selection to the range inside exception marker. const allowedToDelete = marker.getRange().getIntersection( selection.getFirstRange() ); - // Because the DeleteCommand uses the same selection to set selection after calling model.deleteContent() it is better to modify - // the argument selection. The only problem is when that when DocumentSelection is passed (or used internally) as it does allows - // to be modified. - // Instead we change the arguments passed to `deleteContent()` method when document selection was passed. + // 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' ) ) { - args[ 0 ] = editor.model.createSelection( allowedToDelete ); + editor.model.change( writer => { + writer.setSelection( allowedToDelete ); + } ); } - // We need to modify selection passed to deleteContent if it is an instance of selection because DeleteCommand uses passed - // selection to set selection afterwards. Since we modifying this here the selection set after the delete content will be invalid. + // ... or by modifying passed selection instance directly. else { selection.setTo( allowedToDelete ); }