Merge pull request #1025 from ckeditor/t/1024

Feature: Hide caret when an editor is read-only. EditingControler is observable from now. Observable property isReadOnly is added to the ViewDocument and EditingController. Closes #1024. Closes ckeditor/ckeditor5#503.

Author: Piotr Jasiun

src/controller/editingcontroller.js

@@ -22,12 +22,15 @@ import {
 	clearFakeSelection
 } from '../conversion/model-selection-to-view-converters';
 
-import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
+import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
+import mix from '@ckeditor/ckeditor5-utils/src/mix';
 
 /**
  * Controller for the editing pipeline. The editing pipeline controls {@link ~EditingController#model model} rendering,
  * including selection handling. It also creates {@link ~EditingController#view view document} which build a
  * browser-independent virtualization over the DOM elements. Editing controller also attach default converters.
+ *
+ * @mixes module:utils/observablemixin~ObservableMixin
  */
 export default class EditingController {
 	/**
@@ -60,6 +63,19 @@ export default class EditingController {
 		 */
 		this.mapper = new Mapper();
 
+		/**
+		 * Defines whether controller is in read-only mode.
+		 *
+		 * When controller is read-ony then {module:engine/view/document~Document view document} is read-only as well.
+		 *
+		 * @observable
+		 * @member {Boolean} #isReadOnly
+		 */
+		this.set( 'isReadOnly', false );
+
+		// When controller is read-only the view document is read-only as well.
+		this.view.bind( 'isReadOnly' ).to( this );
+
 		/**
 		 * Model to view conversion dispatcher, which converts changes from the model to
 		 * {@link #view editing view}.
@@ -80,39 +96,30 @@ export default class EditingController {
 			viewSelection: this.view.selection
 		} );
 
-		/**
-		 * Property keeping all listenters attached by controller on other objects, so it can
-		 * stop listening on {@link #destroy}.
-		 *
-		 * @private
-		 * @member {utils.EmitterMixin} #_listener
-		 */
-		this._listener = Object.create( EmitterMixin );
-
 		// Convert changes in model to view.
-		this._listener.listenTo( this.model, 'change', ( evt, type, changes ) => {
+		this.listenTo( this.model, 'change', ( evt, type, changes ) => {
 			this.modelToView.convertChange( type, changes );
 		}, { priority: 'low' } );
 
 		// Convert model selection to view.
-		this._listener.listenTo( this.model, 'changesDone', () => {
+		this.listenTo( this.model, 'changesDone', () => {
 			const selection = this.model.selection;
 
 			this.modelToView.convertSelection( selection );
 			this.view.render();
 		}, { priority: 'low' } );
 
 		// Convert model markers changes.
-		this._listener.listenTo( this.model.markers, 'add', ( evt, marker ) => {
+		this.listenTo( this.model.markers, 'add', ( evt, marker ) => {
 			this.modelToView.convertMarker( 'addMarker', marker.name, marker.getRange() );
 		} );
 
-		this._listener.listenTo( this.model.markers, 'remove', ( evt, marker ) => {
+		this.listenTo( this.model.markers, 'remove', ( evt, marker ) => {
 			this.modelToView.convertMarker( 'removeMarker', marker.name, marker.getRange() );
 		} );
 
 		// Convert view selection to model.
-		this._listener.listenTo( this.view, 'selectionChange', convertSelectionChange( this.model, this.mapper ) );
+		this.listenTo( this.view, 'selectionChange', convertSelectionChange( this.model, this.mapper ) );
 
 		// Attach default content converters.
 		this.modelToView.on( 'insert:$text', insertText(), { priority: 'lowest' } );
@@ -158,6 +165,8 @@ export default class EditingController {
 	 */
 	destroy() {
 		this.view.destroy();
-		this._listener.stopListening();
+		this.stopListening();
 	}
 }
+
+mix( EditingController, ObservableMixin );

src/view/document.js

@@ -79,6 +79,16 @@ export default class Document {
 		 */
 		this.roots = new Map();
 
+		/**
+		 * Defines whether document is in read-only mode.
+		 *
+		 * When document is read-ony then all roots are read-only as well and caret placed inside this root is hidden.
+		 *
+		 * @observable
+		 * @member {Boolean} #isReadOnly
+		 */
+		this.set( 'isReadOnly', false );
+
 		/**
 		 * True if document is focused.
 		 *
@@ -98,7 +108,7 @@ export default class Document {
 		 * @member {module:engine/view/renderer~Renderer} module:engine/view/document~Document#renderer
 		 */
 		this.renderer = new Renderer( this.domConverter, this.selection );
-		this.renderer.bind( 'isFocused' ).to( this, 'isFocused' );
+		this.renderer.bind( 'isFocused' ).to( this );
 
 		/**
 		 * Map of registered {@link module:engine/view/observer/observer~Observer observers}.

src/view/editableelement.js

@@ -18,8 +18,10 @@ const documentSymbol = Symbol( 'document' );
  * Editable element which can be a {@link module:engine/view/rooteditableelement~RootEditableElement root}
  * or nested editable area in the editor.
  *
+ * Editable is automatically read-only when its {module:engine/view/document~Document Document} is read-only.
+ *
  * @extends module:engine/view/containerelement~ContainerElement
- * @mixes module:utils/observablemixin~ObservaleMixin
+ * @mixes module:utils/observablemixin~ObservableMixin
  */
 export default class EditableElement extends ContainerElement {
 	/**
@@ -74,6 +76,8 @@ export default class EditableElement extends ContainerElement {
 
 		this.setCustomProperty( documentSymbol, document );
 
+		this.bind( 'isReadOnly' ).to( document );
+
 		this.bind( 'isFocused' ).to(
 			document,
 			'isFocused',

src/view/observer/selectionobserver.js

@@ -133,7 +133,10 @@ export default class SelectionObserver extends Observer {
 	 * @param {Document} domDocument DOM document.
 	 */
 	_handleSelectionChange( domDocument ) {
-		if ( !this.isEnabled || !this.document.isFocused ) {
+		// Selection is handled when document is not focused but is read-only. This is because in read-only
+		// mode contenteditable is set as false and editor won't receive focus but we still need to know
+		// selection position.
+		if ( !this.isEnabled || ( !this.document.isFocused && !this.document.isReadOnly ) ) {
 			return;
 		}
 

tests/controller/editingcontroller.js

@@ -29,20 +29,48 @@ import { getData as getViewData } from '../../src/dev-utils/view';
 
 describe( 'EditingController', () => {
 	describe( 'constructor()', () => {
-		it( 'should create controller with properties', () => {
-			const model = new ModelDocument();
-			const editing = new EditingController( model );
+		let model, editing;
+
+		beforeEach( () => {
+			model = new ModelDocument();
+			editing = new EditingController( model );
+		} );
+
+		afterEach( () => {
+			editing.destroy();
+		} );
 
+		it( 'should create controller with properties', () => {
 			expect( editing ).to.have.property( 'model' ).that.equals( model );
 			expect( editing ).to.have.property( 'view' ).that.is.instanceof( ViewDocument );
 			expect( editing ).to.have.property( 'mapper' ).that.is.instanceof( Mapper );
 			expect( editing ).to.have.property( 'modelToView' ).that.is.instanceof( ModelConversionDispatcher );
+			expect( editing ).to.have.property( 'isReadOnly' ).that.is.false;
 
 			editing.destroy();
 		} );
+
+		it( 'should be observable', () => {
+			const spy = sinon.spy();
+
+			editing.on( 'change:foo', spy );
+			editing.set( 'foo', 'bar' );
+
+			sinon.assert.calledOnce( spy );
+		} );
+
+		it( 'should bind view#isReadOnly to controller#isReadOnly', () => {
+			editing.isReadOnly = false;
+
+			expect( editing.view.isReadOnly ).to.false;
+
+			editing.isReadOnly = true;
+
+			expect( editing.view.isReadOnly ).to.true;
+		} );
 	} );
 
-	describe( 'createRoot', () => {
+	describe( 'createRoot()', () => {
 		let model, modelRoot, editing;
 
 		beforeEach( () => {
@@ -377,7 +405,7 @@ describe( 'EditingController', () => {
 		} );
 	} );
 
-	describe( 'destroy', () => {
+	describe( 'destroy()', () => {
 		it( 'should remove listenters', () => {
 			const model = new ModelDocument();
 			model.createRoot();

tests/view/_utils/createdocumentmock.js

@@ -14,6 +14,7 @@ import Selection from '../../../src/view/selection';
 export default function createDocumentMock() {
 	const doc = Object.create( ObservableMixin );
 	doc.set( 'isFocused', false );
+	doc.set( 'isReadOnly', false );
 	doc.selection = new Selection();
 
 	return doc;

tests/view/document/document.js

@@ -71,9 +71,10 @@ describe( 'Document', () => {
 		it( 'should create Document with all properties', () => {
 			expect( count( viewDocument.domRoots ) ).to.equal( 0 );
 			expect( count( viewDocument.roots ) ).to.equal( 0 );
-			expect( viewDocument ).to.have.property( 'renderer' ).that.is.instanceOf( Renderer );
-			expect( viewDocument ).to.have.property( 'domConverter' ).that.is.instanceOf( DomConverter );
-			expect( viewDocument ).to.have.property( 'isFocused' ).that.is.false;
+			expect( viewDocument ).to.have.property( 'renderer' ).to.instanceOf( Renderer );
+			expect( viewDocument ).to.have.property( 'domConverter' ).to.instanceOf( DomConverter );
+			expect( viewDocument ).to.have.property( 'isReadOnly' ).to.false;
+			expect( viewDocument ).to.have.property( 'isFocused' ).to.false;
 		} );
 
 		it( 'should add default observers', () => {

tests/view/editableelement.js

@@ -151,6 +151,19 @@ describe( 'EditableElement', () => {
 
 			expect( isReadOnlySpy.calledOnce ).to.be.true;
 		} );
+
+		it( 'should be bound to the document#isReadOnly', () => {
+			const root = new RootEditableElement( 'div' );
+			root.document = createDocumentMock();
+
+			root.document.isReadOnly = false;
+
+			expect( root.isReadOnly ).to.false;
+
+			root.document.isReadOnly = true;
+
+			expect( root.isReadOnly ).to.true;
+		} );
 	} );
 
 	describe( 'getDocument', () => {

tests/view/observer/selectionobserver.js

@@ -134,6 +134,26 @@ describe( 'SelectionObserver', () => {
 		changeDomSelection();
 	} );
 
+	it( 'should fired if there is no focus but document is read-only', done => {
+		const spy = sinon.spy();
+
+		viewDocument.isFocused = false;
+		viewDocument.isReadOnly = true;
+
+		// changeDomSelection() may focus the editable element (happens on Chrome)
+		// so cancel this because it sets the isFocused flag.
+		viewDocument.on( 'focus', evt => evt.stop(), { priority: 'highest' } );
+
+		viewDocument.on( 'selectionChange', spy );
+
+		setTimeout( () => {
+			sinon.assert.calledOnce( spy );
+			done();
+		}, 70 );
+
+		changeDomSelection();
+	} );
+
 	it( 'should warn and not enter infinite loop', () => {
 		// Selectionchange event is called twice per `changeDomSelection()` execution.
 		let counter = 35;

tests/view/utils-tests/createdocumentmock.js

@@ -6,19 +6,26 @@
 import createDocumentMock from '../../../tests/view/_utils/createdocumentmock';
 
 describe( 'createDocumentMock', () => {
-	it( 'should create document mock', done => {
+	it( 'should create document mock', () => {
 		const docMock = createDocumentMock();
 		const rootMock = {};
 
+		const isFocusedSpy = sinon.spy();
+		const isReadOnlySpy = sinon.spy();
+
 		docMock.on( 'change:selectedEditable', ( evt, key, value ) => {
 			expect( value ).to.equal( rootMock );
 		} );
 
-		docMock.on( 'change:isFocused', ( evt, key, value ) => {
-			expect( value ).to.be.true;
-			done();
-		} );
+		docMock.on( 'change:isFocused', isFocusedSpy );
+		docMock.on( 'change:isReadOnly', isReadOnlySpy );
 
 		docMock.isFocused = true;
+		docMock.isReadOnly = true;
+
+		sinon.assert.calledOnce( isFocusedSpy );
+		expect( isFocusedSpy.lastCall.args[ 2 ] ).to.true;
+		sinon.assert.calledOnce( isReadOnlySpy );
+		expect( isReadOnlySpy.lastCall.args[ 2 ] ).to.true;
 	} );
 } );