Browse Source

Fixed LiveSelection after rebasing.

Maciej Bukowski 8 years ago
parent
commit
ae5f0fefe1

+ 4 - 2
packages/ckeditor5-engine/src/conversion/model-selection-to-view-converters.js

@@ -35,12 +35,14 @@ export function convertRangeSelection() {
 			return;
 		}
 
-		conversionApi.viewSelection.setTo( null );
+		const viewRanges = [];
 
 		for ( const range of selection.getRanges() ) {
 			const viewRange = conversionApi.mapper.toViewRange( range );
-			conversionApi.viewSelection.setTo( viewRange, selection.isBackward );
+			viewRanges.push( viewRange );
 		}
+
+		conversionApi.viewSelection.setTo( viewRanges, selection.isBackward );
 	};
 }
 

+ 40 - 10
packages/ckeditor5-engine/src/model/liveselection.js

@@ -60,22 +60,48 @@ export default class LiveSelection extends Selection {
 		 */
 		this._attributePriority = new Map();
 
-		this.listenTo( this._document, 'change', ( evt, type, changes, batch ) => {
+		// Add events that will ensure selection correctness.
+		this.on( 'change:range', () => {
+			for ( const range of this.getRanges() ) {
+				if ( !this._document._validateSelectionRange( range ) ) {
+					/**
+					 * Range from {@link module:engine/model/documentselection~DocumentSelection document selection}
+					 * starts or ends at incorrect position.
+					 *
+					 * @error document-selection-wrong-position
+					 * @param {module:engine/model/range~Range} range
+					 */
+					throw new CKEditorError(
+						'document-selection-wrong-position: Range from document selection starts or ends at incorrect position.',
+						{ range }
+					);
+				}
+			}
+		} );
+
+		this.listenTo( this._model, 'applyOperation', ( evt, args ) => {
+			const operation = args[ 0 ];
+
+			if ( !operation.isDocumentOperation ) {
+				return;
+			}
+
 			// Whenever attribute operation is performed on document, update selection attributes.
 			// This is not the most efficient way to update selection attributes, but should be okay for now.
-			if ( attrOpTypes.has( type ) ) {
+			if ( attrOpTypes.has( operation.type ) ) {
 				this._updateAttributes( false );
 			}
 
+			const batch = operation.delta.batch;
+
 			// Batch may not be passed to the document#change event in some tests.
 			// See https://github.com/ckeditor/ckeditor5-engine/issues/1001#issuecomment-314202352
-			// Ignore also transparent batches because they are... transparent.
-			if ( batch && batch.type !== 'transparent' ) {
+			if ( batch ) {
 				// Whenever element which had selection's attributes stored in it stops being empty,
 				// the attributes need to be removed.
-				clearAttributesStoredInElement( changes, this._model, batch );
+				clearAttributesStoredInElement( operation, this._model, batch );
 			}
-		} );
+		}, { priority: 'low' } );
 	}
 
 	/**
@@ -681,11 +707,15 @@ function getAttrsIfCharacter( node ) {
 }
 
 // Removes selection attributes from element which is not empty anymore.
-function clearAttributesStoredInElement( changes, model, batch ) {
-	const changeParent = changes.range && changes.range.start.parent;
+function clearAttributesStoredInElement( operation, model, batch ) {
+	let changeParent = null;
+
+	if ( operation.type == 'insert' ) {
+		changeParent = operation.position.parent;
+	} else if ( operation.type == 'move' || operation.type == 'reinsert' || operation.type == 'remove' ) {
+		changeParent = operation.getMovedRangeStart().parent;
+	}
 
-	// `changes.range` is not set in case of rename, root and marker operations.
-	// None of them may lead to the element becoming non-empty.
 	if ( !changeParent || changeParent.isEmpty ) {
 		return;
 	}

+ 15 - 15
packages/ckeditor5-engine/tests/dev-utils/model.js

@@ -21,7 +21,7 @@ describe( 'model test utils', () => {
 		root = document.createRoot();
 		selection = document.selection;
 		sandbox = sinon.sandbox.create();
-		selection.removeAllRanges();
+		selection._setTo( null );
 
 		model.schema.register( 'a', {
 			allowWhere: '$text',
@@ -64,7 +64,7 @@ describe( 'model test utils', () => {
 		it( 'should use stringify method with selection', () => {
 			const stringifySpy = sandbox.spy( getData, '_stringify' );
 			root.appendChildren( new Element( 'b', null, new Text( 'btext' ) ) );
-			document.selection.setTo( Range.createFromParentsAndOffsets( root, 0, root, 1 ) );
+			document.selection._setTo( Range.createFromParentsAndOffsets( root, 0, root, 1 ) );
 
 			expect( getData( model ) ).to.equal( '[<b>btext</b>]' );
 			sinon.assert.calledOnce( stringifySpy );
@@ -278,7 +278,7 @@ describe( 'model test utils', () => {
 
 			it( 'writes selection in an empty root', () => {
 				const root = document.createRoot( '$root', 'empty' );
-				selection.setTo( root );
+				selection._setTo( root );
 
 				expect( stringify( root, selection ) ).to.equal(
 					'[]'
@@ -286,7 +286,7 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes selection collapsed in an element', () => {
-				selection.setTo( root );
+				selection._setTo( root );
 
 				expect( stringify( root, selection ) ).to.equal(
 					'[]<a></a>foo<$text bold="true">bar</$text><b></b>'
@@ -294,7 +294,7 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes selection collapsed in a text', () => {
-				selection.setTo( root, 3 );
+				selection._setTo( root, 3 );
 
 				expect( stringify( root, selection ) ).to.equal(
 					'<a></a>fo[]o<$text bold="true">bar</$text><b></b>'
@@ -302,7 +302,7 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes selection collapsed at the text left boundary', () => {
-				selection.setTo( elA, 'after' );
+				selection._setTo( elA, 'after' );
 
 				expect( stringify( root, selection ) ).to.equal(
 					'<a></a>[]foo<$text bold="true">bar</$text><b></b>'
@@ -310,7 +310,7 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes selection collapsed at the text right boundary', () => {
-				selection.setTo( elB, 'before' );
+				selection._setTo( elB, 'before' );
 
 				expect( stringify( root, selection ) ).to.equal(
 					'<a></a>foo<$text bold="true">bar[]</$text><b></b>'
@@ -318,10 +318,10 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes selection collapsed at the end of the root', () => {
-				selection.setTo( root, 'end' );
+				selection._setTo( root, 'end' );
 
 				// Needed due to https://github.com/ckeditor/ckeditor5-engine/issues/320.
-				selection.clearAttributes();
+				selection._clearAttributes();
 
 				expect( stringify( root, selection ) ).to.equal(
 					'<a></a>foo<$text bold="true">bar</$text><b></b>[]'
@@ -329,7 +329,7 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes selection collapsed selection in a text with attributes', () => {
-				selection.setTo( root, 5 );
+				selection._setTo( root, 5 );
 
 				expect( stringify( root, selection ) ).to.equal(
 					'<a></a>foo<$text bold="true">b[]ar</$text><b></b>'
@@ -337,7 +337,7 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes flat selection containing couple of nodes', () => {
-				selection.setTo(
+				selection._setTo(
 					Range.createFromParentsAndOffsets( root, 0, root, 4 )
 				);
 
@@ -347,7 +347,7 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes flat selection within text', () => {
-				selection.setTo(
+				selection._setTo(
 					Range.createFromParentsAndOffsets( root, 2, root, 3 )
 				);
 
@@ -357,7 +357,7 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes multi-level selection', () => {
-				selection.setTo(
+				selection._setTo(
 					Range.createFromParentsAndOffsets( elA, 0, elB, 0 )
 				);
 
@@ -367,7 +367,7 @@ describe( 'model test utils', () => {
 			} );
 
 			it( 'writes selection when is backward', () => {
-				selection.setTo(
+				selection._setTo(
 					Range.createFromParentsAndOffsets( elA, 0, elB, 0 ),
 					true
 				);
@@ -381,7 +381,7 @@ describe( 'model test utils', () => {
 				const root = document.createRoot( '$root', 'empty' );
 
 				root.appendChildren( new Text( 'நிலைக்கு' ) );
-				selection.setTo( Range.createFromParentsAndOffsets( root, 2, root, 6 ) );
+				selection._setTo( Range.createFromParentsAndOffsets( root, 2, root, 6 ) );
 
 				expect( stringify( root, selection ) ).to.equal( 'நி[லைக்]கு' );
 			} );

+ 1 - 1
packages/ckeditor5-engine/tests/model/document.js

@@ -581,7 +581,7 @@ describe( 'Document', () => {
 			doc.on( 'change', spy );
 
 			model.change( () => {
-				doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 2, root, 2 ) ] );
+				doc.selection._setTo( Range.createFromParentsAndOffsets( root, 2, root, 2 ) );
 			} );
 
 			expect( spy.calledOnce ).to.be.true;

+ 10 - 24
packages/ckeditor5-engine/tests/model/documentselection.js

@@ -972,7 +972,7 @@ describe( 'DocumentSelection', () => {
 				// Dedupe batches by using a map (multiple change events will be fired).
 				const batchTypes = new Map();
 
-				selection._setTo( [ rangeInEmptyP ] );
+				selection._setTo( rangeInEmptyP );
 				selection._setAttribute( 'foo', 'bar' );
 				selection._setAttribute( 'abc', 'bar' );
 
@@ -994,7 +994,7 @@ describe( 'DocumentSelection', () => {
 			} );
 
 			it( 'are removed when any content is moved into', () => {
-				selection._setTo( [ rangeInEmptyP ] );
+				selection._setTo( rangeInEmptyP );
 				selection._setAttribute( 'foo', 'bar' );
 
 				model.change( writer => {
@@ -1085,20 +1085,6 @@ describe( 'DocumentSelection', () => {
 				expect( emptyP.parent ).to.equal( root ); // Just to be sure we're checking the right element.
 			} );
 
-			it( 'are not removed on transparent batches', () => {
-				selection._setTo( [ rangeInEmptyP ] );
-				selection._setAttribute( 'foo', 'bar' );
-
-				model.enqueueChange( 'transparent', writer => {
-					sinon.spy( model, 'enqueueChange' );
-
-					writer.insertText( 'x', rangeInEmptyP.start );
-
-					expect( model.enqueueChange.called ).to.be.false;
-					expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' );
-				} );
-			} );
-
 			// Rename and some other deltas don't specify range in doc#change event.
 			// So let's see if there's no crash or something.
 			it( 'are not removed on rename', () => {
@@ -1122,11 +1108,11 @@ describe( 'DocumentSelection', () => {
 		root.appendChildren( '\uD83D\uDCA9' );
 
 		expect( () => {
-			doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 0, root, 1 ) ] );
+			doc.selection._setTo( Range.createFromParentsAndOffsets( root, 0, root, 1 ) );
 		} ).to.throw( CKEditorError, /document-selection-wrong-position/ );
 
 		expect( () => {
-			doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 2 ) ] );
+			doc.selection._setTo( Range.createFromParentsAndOffsets( root, 1, root, 2 ) );
 		} ).to.throw( CKEditorError, /document-selection-wrong-position/ );
 	} );
 
@@ -1135,27 +1121,27 @@ describe( 'DocumentSelection', () => {
 		root.appendChildren( 'foo̻̐ͩbar' );
 
 		expect( () => {
-			doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 3, root, 9 ) ] );
+			doc.selection._setTo( Range.createFromParentsAndOffsets( root, 3, root, 9 ) );
 		} ).to.throw( CKEditorError, /document-selection-wrong-position/ );
 
 		expect( () => {
-			doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 4, root, 9 ) ] );
+			doc.selection._setTo( Range.createFromParentsAndOffsets( root, 4, root, 9 ) );
 		} ).to.throw( CKEditorError, /document-selection-wrong-position/ );
 
 		expect( () => {
-			doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 5, root, 9 ) ] );
+			doc.selection._setTo( Range.createFromParentsAndOffsets( root, 5, root, 9 ) );
 		} ).to.throw( CKEditorError, /document-selection-wrong-position/ );
 
 		expect( () => {
-			doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 3 ) ] );
+			doc.selection._setTo( Range.createFromParentsAndOffsets( root, 1, root, 3 ) );
 		} ).to.throw( CKEditorError, /document-selection-wrong-position/ );
 
 		expect( () => {
-			doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 4 ) ] );
+			doc.selection._setTo( Range.createFromParentsAndOffsets( root, 1, root, 4 ) );
 		} ).to.throw( CKEditorError, /document-selection-wrong-position/ );
 
 		expect( () => {
-			doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 5 ) ] );
+			doc.selection._setTo( Range.createFromParentsAndOffsets( root, 1, root, 5 ) );
 		} ).to.throw( CKEditorError, /document-selection-wrong-position/ );
 	} );
 } );