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

Internal: Prevented from rendering view when markers are converted.

Oskar Wróbel 8 лет назад
Родитель
Сommit
277e8def9a

+ 7 - 8
packages/ckeditor5-engine/src/controller/editingcontroller.js

@@ -9,6 +9,7 @@
 
 
 import RootEditableElement from '../view/rooteditableelement';
 import RootEditableElement from '../view/rooteditableelement';
 import View from '../view/view';
 import View from '../view/view';
+import ViewWriter from '../view/writer';
 import Mapper from '../conversion/mapper';
 import Mapper from '../conversion/mapper';
 import DowncastDispatcher from '../conversion/downcastdispatcher';
 import DowncastDispatcher from '../conversion/downcastdispatcher';
 import {
 import {
@@ -106,6 +107,10 @@ export default class EditingController {
 		// `removedMarkers` keeps information which markers already has been removed to prevent removing them twice.
 		// `removedMarkers` keeps information which markers already has been removed to prevent removing them twice.
 		const removedMarkers = new Set();
 		const removedMarkers = new Set();
 
 
+		// We don't want to render view in the middle of the `Model#change` block, so we need to create view writer
+		// manually instead of using `View#change` block. See https://github.com/ckeditor/ckeditor5-engine/issues/1323.
+		const viewWriter = new ViewWriter( this.view.document );
+
 		this.listenTo( model, 'applyOperation', ( evt, args ) => {
 		this.listenTo( model, 'applyOperation', ( evt, args ) => {
 			// Before operation is applied...
 			// Before operation is applied...
 			const operation = args[ 0 ];
 			const operation = args[ 0 ];
@@ -121,10 +126,7 @@ export default class EditingController {
 				if ( _operationAffectsMarker( operation, marker ) ) {
 				if ( _operationAffectsMarker( operation, marker ) ) {
 					// And if the operation in any way modifies the marker, remove the marker from the view.
 					// And if the operation in any way modifies the marker, remove the marker from the view.
 					removedMarkers.add( marker.name );
 					removedMarkers.add( marker.name );
-					this.view.change( writer => {
-						this.downcastDispatcher.convertMarkerRemove( marker.name, markerRange, writer );
-					} );
-
+					this.downcastDispatcher.convertMarkerRemove( marker.name, markerRange, viewWriter );
 					// TODO: This stinks but this is the safest place to have this code.
 					// TODO: This stinks but this is the safest place to have this code.
 					this.model.document.differ.bufferMarkerChange( marker.name, markerRange, markerRange );
 					this.model.document.differ.bufferMarkerChange( marker.name, markerRange, markerRange );
 				}
 				}
@@ -135,10 +137,7 @@ export default class EditingController {
 		this.listenTo( model.markers, 'update', ( evt, marker, oldRange ) => {
 		this.listenTo( model.markers, 'update', ( evt, marker, oldRange ) => {
 			if ( oldRange && !removedMarkers.has( marker.name ) ) {
 			if ( oldRange && !removedMarkers.has( marker.name ) ) {
 				removedMarkers.add( marker.name );
 				removedMarkers.add( marker.name );
-
-				this.view.change( writer => {
-					this.downcastDispatcher.convertMarkerRemove( marker.name, oldRange, writer );
-				} );
+				this.downcastDispatcher.convertMarkerRemove( marker.name, oldRange, viewWriter );
 			}
 			}
 		} );
 		} );
 
 

+ 47 - 0
packages/ckeditor5-engine/tests/tickets/1323.js

@@ -0,0 +1,47 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import EditingController from '../../src/controller/editingcontroller';
+
+import Model from '../../src/model/model';
+import ModelText from '../../src/model/text';
+import ModelRange from '../../src/model/range';
+
+import MarkerOperation from '../../src/model/operation/markeroperation';
+import { wrapInDelta } from '../model/_utils/utils';
+
+describe( 'Bug ckeditor5-engine@1323', () => {
+	describe( 'constructor()', () => {
+		let model, editing, root, range;
+
+		beforeEach( () => {
+			model = new Model();
+			editing = new EditingController( model );
+			root = model.document.createRoot();
+			root.appendChildren( new ModelText( 'foo' ) );
+			range = ModelRange.createFromParentsAndOffsets( root, 0, root, 0 );
+		} );
+
+		afterEach( () => {
+			editing.destroy();
+		} );
+
+		it( 'should not fire view#render event before initial model#change block is finished', () => {
+			const spy = sinon.spy();
+
+			editing.view.on( 'render', spy );
+
+			model.change( () => {
+				// Add marker.
+				model.applyOperation( wrapInDelta( new MarkerOperation( 'name', null, range, model.markers, 0 ) ) );
+
+				// Remove marker.
+				model.applyOperation( wrapInDelta( new MarkerOperation( 'name', range, null, model.markers, 1 ) ) );
+
+				sinon.assert.notCalled( spy );
+			} );
+		} );
+	} );
+} );