Merge pull request #10691 from ckeditor/ck/10493-affects-content

Feature (core): Introduced the `Command#affectsData` flag to indicate whether a given command should stay enabled in editor modes with restricted write permissions (e.g. read-only mode). Closes #10670.

Fix (restricted-editing): The feature should work properly in editor modes with restricted write permissions (e.g. read-only mode). Closes #10634.

Fix (select-all): The `SelectAllCommand` should work properly in editor modes with restricted write permissions (e.g. read-only mode). Closes #10635.

Fix (table): The `SelectColumnCommand` and `SelectRowCommand` should work properly in editor modes with restricted write permissions (e.g. read-only mode). See #10635.

Fix (find-and-replace): `FindCommand` and `FindNextCommand` should work properly in editor modes with restricted write permissions (e.g. read-only mode). Closes #10636.

docs/framework/guides/architecture/core-editor-architecture.md

@@ -150,6 +150,25 @@ enableBold();
 
 The command will now be disabled as long as you do not {@link module:utils/emittermixin~EmitterMixin#off off} this listener, regardless of how many times `someCommand.refresh()` is called.
 
+By default, editor commands are disabled when the editor is in {@link module:core/editor/editor~Editor#isReadOnly read-only} mode. However, if your command does not change the editor data and you want it to stay enabled in the read-only mode, you can set the {@link module:core/command~Command#affectsData `affectsData`} flag to `false`:
+
+```js
+class MyAlwaysEnabledCommand extends Command {
+	constructor( editor ) {
+		super( editor );
+
+		// This command will remain enabled even when the editor is read-only.
+		this.affectsData = false;
+	}
+}
+```
+
+The {@link module:core/command~Command#affectsData `affectsData`} flag will also affect the command in {@link features/read-only#related-features other editor modes} that restrict user write permissions.
+
+<info-box>
+	The `affectsData` flag is `true` by default for all editor commands and, unless your command should be enabled when the editor is read-only, you do not need to change it. Also, please keep in mind the flag is immutable during the lifetime of the editor.
+</info-box>
+
 ## Event system and observables
 
 CKEditor 5 has an event-based architecture so you can find {@link module:utils/emittermixin~EmitterMixin} and {@link module:utils/observablemixin~ObservableMixin} mixed all over the place. Both mechanisms allow for decoupling the code and make it extensible.

packages/ckeditor5-core/src/command.js

@@ -20,7 +20,8 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix';
  * Instances of registered commands can be retrieved from {@link module:core/editor/editor~Editor#commands `editor.commands`}.
  * The easiest way to execute a command is through {@link module:core/editor/editor~Editor#execute `editor.execute()`}.
  *
- * By default commands are disabled when the editor is in {@link module:core/editor/editor~Editor#isReadOnly read-only} mode.
+ * By default, commands are disabled when the editor is in {@link module:core/editor/editor~Editor#isReadOnly read-only} mode
+ * but commands with the {@link module:core/command~Command#affectsData `affectsData`} flag set to `false` will not be disabled.
  *
  * @mixes module:utils/observablemixin~ObservableMixin
  */
@@ -96,6 +97,21 @@ export default class Command {
 		 */
 		this.set( 'isEnabled', false );
 
+		/**
+		 * A flag indicating whether a command execution changes the editor data or not.
+		 *
+		 * Commands with `affectsData` set to `false` will not be automatically disabled in
+		 * {@link module:core/editor/editor~Editor#isReadOnly read-only mode} and
+		 * {@glink features/read-only#related-features other editor modes} with restricted user write permissions.
+		 *
+		 * **Note:** You do not have to set it for your every command. It will be `true` by default.
+		 *
+		 * @readonly
+		 * @default true
+		 * @member {Boolean} #affectsData
+		 */
+		this.affectsData = true;
+
 		/**
 		 * Holds identifiers for {@link #forceDisabled} mechanism.
 		 *
@@ -119,7 +135,7 @@ export default class Command {
 
 		// By default commands are disabled when the editor is in read-only mode.
 		this.listenTo( editor, 'change:isReadOnly', ( evt, name, value ) => {
-			if ( value ) {
+			if ( value && this.affectsData ) {
 				this.forceDisabled( 'readOnlyMode' );
 			} else {
 				this.clearForceDisabled( 'readOnlyMode' );

packages/ckeditor5-core/tests/command.js

@@ -34,6 +34,10 @@ describe( 'Command', () => {
 			expect( command.isEnabled ).to.be.false;
 		} );
 
+		it( 'sets the affectsData property', () => {
+			expect( command ).to.have.property( 'affectsData', true );
+		} );
+
 		it( 'adds a listener which refreshes the command on editor.model.Document#event:change', () => {
 			sinon.spy( command, 'refresh' );
 
@@ -66,7 +70,8 @@ describe( 'Command', () => {
 			expect( spy.calledOnce ).to.be.true;
 		} );
 
-		it( 'is always falsy when the editor is in read-only mode', () => {
+		it( 'is falsy when the editor is in read-only mode and command affects data', () => {
+			command.affectsData = true;
 			editor.isReadOnly = false;
 			command.isEnabled = true;
 
@@ -86,6 +91,27 @@ describe( 'Command', () => {
 			expect( command.isEnabled ).to.true;
 		} );
 
+		it( 'doesn\'t depend on the editor read-only mode when command doesn\'t affect data', () => {
+			command.affectsData = false;
+			editor.isReadOnly = false;
+			command.isEnabled = true;
+
+			editor.isReadOnly = true;
+
+			// Is true.
+			expect( command.isEnabled ).to.true;
+
+			command.refresh();
+
+			// Still true.
+			expect( command.isEnabled ).to.true;
+
+			editor.isReadOnly = false;
+
+			// And is back to true.
+			expect( command.isEnabled ).to.true;
+		} );
+
 		it( 'is observable when is overridden', () => {
 			editor.isReadOnly = false;
 			command.isEnabled = true;

packages/ckeditor5-find-and-replace/src/findcommand.js

@@ -28,18 +28,16 @@ export default class FindCommand extends Command {
 		// The find command is always enabled.
 		this.isEnabled = true;
 
+		// It does not affect data so should be enabled in read-only mode.
+		this.affectsData = false;
+
 		/**
 		 * The find and replace state object used for command operations.
 		 *
 		 * @private
 		 * @member {module:find-and-replace/findandreplacestate~FindAndReplaceState} #_state
 		 */
 		this._state = state;
-
-		// Do not block the command if the editor goes into the read-only mode as it does not impact the data. See #9975.
-		this.listenTo( editor, 'change:isReadOnly', () => {
-			this.clearForceDisabled( 'readOnlyMode' );
-		} );
 	}
 
 	/**

packages/ckeditor5-find-and-replace/src/findnextcommand.js

@@ -26,6 +26,9 @@ export default class FindNextCommand extends Command {
 	constructor( editor, state ) {
 		super( editor );
 
+		// It does not affect data so should be enabled in read-only mode.
+		this.affectsData = false;
+
 		/**
 		 * The find and replace state object used for command operations.
 		 *
@@ -39,11 +42,6 @@ export default class FindNextCommand extends Command {
 		this.listenTo( this._state.results, 'change', () => {
 			this.isEnabled = this._state.results.length > 1;
 		} );
-
-		// Do not block the command if the editor goes into the read-only mode as it does not impact the data. See #9975.
-		this.listenTo( editor, 'change:isReadOnly', () => {
-			this.clearForceDisabled( 'readOnlyMode' );
-		} );
 	}
 
 	/**

packages/ckeditor5-find-and-replace/tests/findcommand.js

@@ -27,6 +27,17 @@ describe( 'FindCommand', () => {
 		return editor.destroy();
 	} );
 
+	describe( 'constructor()', () => {
+		it( 'sets public properties', () => {
+			expect( command ).to.have.property( 'isEnabled', true );
+			expect( command ).to.have.property( 'affectsData', false );
+		} );
+
+		it( 'sets state property', () => {
+			expect( command ).to.have.property( '_state', editor.plugins.get( 'FindAndReplaceEditing' ).state );
+		} );
+	} );
+
 	describe( 'isEnabled', () => {
 		it( 'should be enabled in empty document', () => {
 			setData( model, '[]' );
@@ -56,12 +67,6 @@ describe( 'FindCommand', () => {
 		} );
 	} );
 
-	describe( 'state', () => {
-		it( 'is set to plugin\'s state', () => {
-			expect( command._state ).to.equal( editor.plugins.get( 'FindAndReplaceEditing' ).state );
-		} );
-	} );
-
 	describe( 'execute()', () => {
 		describe( 'with string passed', () => {
 			it( 'places markers correctly in the model', () => {

packages/ckeditor5-find-and-replace/tests/findnextcommand.js

@@ -26,6 +26,17 @@ describe( 'FindNextCommand', () => {
 		return editor.destroy();
 	} );
 
+	describe( 'constructor()', () => {
+		it( 'sets public properties', () => {
+			expect( command ).to.have.property( 'isEnabled', false );
+			expect( command ).to.have.property( 'affectsData', false );
+		} );
+
+		it( 'sets state property', () => {
+			expect( command ).to.have.property( '_state', editor.plugins.get( 'FindAndReplaceEditing' ).state );
+		} );
+	} );
+
 	describe( 'isEnabled', () => {
 		it( 'should be disabled in empty document', () => {
 			setData( model, '[]' );
@@ -90,12 +101,6 @@ describe( 'FindNextCommand', () => {
 		} );
 	} );
 
-	describe( '_state', () => {
-		it( 'is set to plugin\'s state', () => {
-			expect( command._state ).to.equal( editor.plugins.get( 'FindAndReplaceEditing' ).state );
-		} );
-	} );
-
 	describe( 'execute()', () => {
 		it( 'moves forward from the first match', () => {
 			setData( model, '<paragraph>[]Foo bar baz. Bam bar bom. bar bar</paragraph>' );

packages/ckeditor5-find-and-replace/tests/findpreviouscommand.js

@@ -26,6 +26,17 @@ describe( 'FindPreviousCommand', () => {
 		return editor.destroy();
 	} );
 
+	describe( 'constructor()', () => {
+		it( 'sets public properties', () => {
+			expect( command ).to.have.property( 'isEnabled', false );
+			expect( command ).to.have.property( 'affectsData', false );
+		} );
+
+		it( 'sets state property', () => {
+			expect( command ).to.have.property( '_state', editor.plugins.get( 'FindAndReplaceEditing' ).state );
+		} );
+	} );
+
 	describe( 'isEnabled', () => {
 		it( 'should be enabled in empty document', () => {
 			setData( model, '[]' );
@@ -89,12 +100,6 @@ describe( 'FindPreviousCommand', () => {
 		} );
 	} );
 
-	describe( 'state', () => {
-		it( 'is set to plugin\'s state', () => {
-			expect( command._state ).to.equal( editor.plugins.get( 'FindAndReplaceEditing' ).state );
-		} );
-	} );
-
 	describe( 'execute()', () => {
 		it( 'moves backward from the first match', () => {
 			setData( model, '<paragraph>[]Foo bar baz. Bam bar bom.</paragraph>' );

packages/ckeditor5-restricted-editing/src/restrictededitingmodeediting.js

@@ -54,7 +54,7 @@ export default class RestrictedEditingModeEditing extends Plugin {
 		 * @type {Set.<String>}
 		 * @private
 		 */
-		this._alwaysEnabled = new Set( [ 'undo', 'redo', 'goToPreviousRestrictedEditingException', 'goToNextRestrictedEditingException' ] );
+		this._alwaysEnabled = new Set( [ 'undo', 'redo' ] );
 
 		/**
 		 * Commands allowed in non-restricted areas.
@@ -241,7 +241,7 @@ export default class RestrictedEditingModeEditing extends Plugin {
 		const model = editor.model;
 		const doc = model.document;
 
-		this._disableCommands( editor );
+		this._disableCommands();
 
 		this.listenTo( doc.selection, 'change', this._checkCommands.bind( this ) );
 		this.listenTo( doc, 'change:data', this._checkCommands.bind( this ) );
@@ -257,7 +257,7 @@ export default class RestrictedEditingModeEditing extends Plugin {
 		const selection = editor.model.document.selection;
 
 		if ( selection.rangeCount > 1 ) {
-			this._disableCommands( editor );
+			this._disableCommands();
 
 			return;
 		}
@@ -280,12 +280,21 @@ export default class RestrictedEditingModeEditing extends Plugin {
 	_enableCommands( marker ) {
 		const editor = this.editor;
 
-		const commands = this._getCommandNamesToToggle( editor, this._allowedInException )
-			.filter( name => this._allowedInException.has( name ) )
-			.filter( filterDeleteCommandsOnMarkerBoundaries( editor.model.document.selection, marker.getRange() ) )
-			.map( name => editor.commands.get( name ) );
+		for ( const [ commandName, command ] of editor.commands ) {
+			if ( !command.affectsData || this._alwaysEnabled.has( commandName ) ) {
+				continue;
+			}
+
+			// Enable ony those commands that are allowed in the exception marker.
+			if ( !this._allowedInException.has( commandName ) ) {
+				continue;
+			}
+
+			// Do not enable 'delete' and 'deleteForward' commands on the exception marker boundaries.
+			if ( isDeleteCommandOnMarkerBoundaries( commandName, editor.model.document.selection, marker.getRange() ) ) {
+				continue;
+			}
 
-		for ( const command of commands ) {
 			command.clearForceDisabled( COMMAND_FORCE_DISABLE_ID );
 		}
 	}
@@ -297,25 +306,15 @@ export default class RestrictedEditingModeEditing extends Plugin {
 	 */
 	_disableCommands() {
 		const editor = this.editor;
-		const commands = this._getCommandNamesToToggle( editor )
-			.map( name => editor.commands.get( name ) );
 
-		for ( const command of commands ) {
+		for ( const [ commandName, command ] of editor.commands ) {
+			if ( !command.affectsData || this._alwaysEnabled.has( commandName ) ) {
+				continue;
+			}
+
 			command.forceDisabled( COMMAND_FORCE_DISABLE_ID );
 		}
 	}
-
-	/**
-	 * Returns command names that should be toggleable.
-	 *
-	 * @param {module:core/editor/editor~Editor} editor
-	 * @returns {Array.<String>}
-	 * @private
-	 */
-	_getCommandNamesToToggle( editor ) {
-		return Array.from( editor.commands.names() )
-			.filter( name => !this._alwaysEnabled.has( name ) );
-	}
 }
 
 // Helper method for executing enabled commands only.
@@ -357,24 +356,22 @@ function getSelectAllHandler( editor ) {
 	};
 }
 
-// Additional filtering rule for enabling "delete" and "deleteForward" commands if selection is on range boundaries:
+// Additional rule for enabling "delete" and "deleteForward" commands if selection is on range boundaries:
 //
 // Does not allow to enable command when selection focus is:
 // - is on marker start - "delete" - to prevent removing content before marker
 // - is on marker end - "deleteForward" - to prevent removing content after marker
-function filterDeleteCommandsOnMarkerBoundaries( selection, markerRange ) {
-	return name => {
-		if ( name == 'delete' && markerRange.start.isEqual( selection.focus ) ) {
-			return false;
-		}
-
-		// Only for collapsed selection - non-collapsed selection that extends over a marker is handled elsewhere.
-		if ( name == 'deleteForward' && selection.isCollapsed && markerRange.end.isEqual( selection.focus ) ) {
-			return false;
-		}
+function isDeleteCommandOnMarkerBoundaries( commandName, selection, markerRange ) {
+	if ( commandName == 'delete' && markerRange.start.isEqual( selection.focus ) ) {
+		return true;
+	}
 
+	// Only for collapsed selection - non-collapsed selection that extends over a marker is handled elsewhere.
+	if ( commandName == 'deleteForward' && selection.isCollapsed && markerRange.end.isEqual( selection.focus ) ) {
 		return true;
-	};
+	}
+
+	return false;
 }
 
 // Ensures that model.deleteContent() does not delete outside exception markers ranges.

packages/ckeditor5-restricted-editing/src/restrictededitingmodenavigationcommand.js

@@ -24,6 +24,9 @@ export default class RestrictedEditingModeNavigationCommand extends Command {
 	constructor( editor, direction ) {
 		super( editor );
 
+		// It does not affect data so should be enabled in read-only mode and in restricted editing mode.
+		this.affectsData = false;
+
 		/**
 		 * The direction of the command. Can be `'forward'` or `'backward'`.
 		 *