浏览代码

Merge branch 'master' into t/ckeditor5/1514

Aleksander Nowodzinski 6 年之前
父节点
当前提交
ab274c5728

+ 20 - 11
packages/ckeditor5-engine/src/model/documentselection.js

@@ -598,7 +598,7 @@ class LiveSelection extends Selection {
 		// @type {Set}
 		// @type {Set}
 		this._overriddenGravityRegister = new Set();
 		this._overriddenGravityRegister = new Set();
 
 
-		// Add events that will ensure selection correctness.
+		// Ensure selection is correct and up to date after each range change.
 		this.on( 'change:range', () => {
 		this.on( 'change:range', () => {
 			for ( const range of this.getRanges() ) {
 			for ( const range of this.getRanges() ) {
 				if ( !this._document._validateSelectionRange( range ) ) {
 				if ( !this._document._validateSelectionRange( range ) ) {
@@ -615,20 +615,22 @@ class LiveSelection extends Selection {
 					);
 					);
 				}
 				}
 			}
 			}
-		} );
 
 
-		this.listenTo( this._document, 'change', ( evt, batch ) => {
-			// Update selection's markers.
 			this._updateMarkers();
 			this._updateMarkers();
-
-			// Update selection's attributes.
 			this._updateAttributes( false );
 			this._updateAttributes( false );
-
-			// Clear selection attributes from element if no longer empty.
-			clearAttributesStoredInElement( this._model, batch );
 		} );
 		} );
 
 
-		this.listenTo( this._model, 'applyOperation', () => {
+		// Update markers data stored by the selection after each marker change.
+		this.listenTo( this._model.markers, 'update', () => this._updateMarkers() );
+
+		// Ensure selection is correct and up to date after each operation.
+		this.listenTo( this._model, 'applyOperation', ( evt, args ) => {
+			const operation = args[ 0 ];
+
+			if ( !operation.isDocumentOperation || operation.type == 'marker' || operation.type == 'rename' || operation.type == 'noop' ) {
+				return;
+			}
+
 			while ( this._fixGraveyardRangesData.length ) {
 			while ( this._fixGraveyardRangesData.length ) {
 				const { liveRange, sourcePosition } = this._fixGraveyardRangesData.shift();
 				const { liveRange, sourcePosition } = this._fixGraveyardRangesData.shift();
 
 
@@ -637,10 +639,17 @@ class LiveSelection extends Selection {
 
 
 			if ( this._hasChangedRange ) {
 			if ( this._hasChangedRange ) {
 				this._hasChangedRange = false;
 				this._hasChangedRange = false;
-
 				this.fire( 'change:range', { directChange: false } );
 				this.fire( 'change:range', { directChange: false } );
 			}
 			}
+
+			this._updateMarkers();
+			this._updateAttributes( false );
 		}, { priority: 'lowest' } );
 		}, { priority: 'lowest' } );
+
+		// Clear selection attributes from element if no longer empty.
+		this.listenTo( this._document, 'change', ( evt, batch ) => {
+			clearAttributesStoredInElement( this._model, batch );
+		} );
 	}
 	}
 
 
 	get isCollapsed() {
 	get isCollapsed() {

+ 0 - 4
packages/ckeditor5-engine/src/model/markercollection.js

@@ -305,10 +305,6 @@ mix( MarkerCollection, EmitterMixin );
  * Then, it can be upcasted back to a marker. Again, use {@link module:engine/conversion/upcasthelpers upcast converters} or
  * Then, it can be upcasted back to a marker. Again, use {@link module:engine/conversion/upcasthelpers upcast converters} or
  * attach a custom converter to {@link module:engine/conversion/upcastdispatcher~UpcastDispatcher#event:element}.
  * attach a custom converter to {@link module:engine/conversion/upcastdispatcher~UpcastDispatcher#event:element}.
  *
  *
- * Another upside of markers is that finding marked part of document is fast and easy. Using attributes to mark some nodes
- * and then trying to find that part of document would require traversing whole document tree. Marker gives instant access
- * to the range which it is marking at the moment.
- *
  * `Marker` instances are created and destroyed only by {@link ~MarkerCollection MarkerCollection}.
  * `Marker` instances are created and destroyed only by {@link ~MarkerCollection MarkerCollection}.
  */
  */
 class Marker {
 class Marker {

+ 16 - 1
packages/ckeditor5-engine/src/model/selection.js

@@ -826,10 +826,25 @@ function isUnvisitedBlockContainer( element, visited ) {
 }
 }
 
 
 // Finds the lowest element in position's ancestors which is a block.
 // Finds the lowest element in position's ancestors which is a block.
+// It will search until first ancestor that is a limit element.
 // Marks all ancestors as already visited to not include any of them later on.
 // Marks all ancestors as already visited to not include any of them later on.
 function getParentBlock( position, visited ) {
 function getParentBlock( position, visited ) {
+	const schema = position.parent.document.model.schema;
+
 	const ancestors = position.parent.getAncestors( { parentFirst: true, includeSelf: true } );
 	const ancestors = position.parent.getAncestors( { parentFirst: true, includeSelf: true } );
-	const block = ancestors.find( element => isUnvisitedBlockContainer( element, visited ) );
+
+	let hasParentLimit = false;
+
+	const block = ancestors.find( element => {
+		// Stop searching after first parent node that is limit element.
+		if ( hasParentLimit ) {
+			return false;
+		}
+
+		hasParentLimit = schema.isLimit( element );
+
+		return !hasParentLimit && isUnvisitedBlockContainer( element, visited );
+	} );
 
 
 	// Mark all ancestors of this position's parent, because find() might've stopped early and
 	// Mark all ancestors of this position's parent, because find() might've stopped early and
 	// the found block may be a child of another block.
 	// the found block may be a child of another block.

+ 64 - 0
packages/ckeditor5-engine/tests/model/documentselection.js

@@ -1141,6 +1141,70 @@ describe( 'DocumentSelection', () => {
 		} );
 		} );
 	} );
 	} );
 
 
+	// https://github.com/ckeditor/ckeditor5-engine/issues/1673
+	describe( 'refreshing selection data', () => {
+		it( 'should be up to date when post fixers are called', done => {
+			model.schema.extend( '$text', { allowAttributes: 'foo' } );
+
+			const p = doc.getRoot().getChild( 0 );
+
+			doc.registerPostFixer( () => {
+				expect( model.document.selection.getAttribute( 'foo' ) ).to.equal( 'bar' );
+				expect( Array.from( model.document.selection.markers, m => m.name ) ).to.deep.equal( [ 'marker' ] );
+				done();
+			} );
+
+			model.change( writer => {
+				writer.insertText( 'abcdef', { foo: 'bar' }, p );
+
+				writer.addMarker( 'marker', {
+					range: writer.createRange(
+						writer.createPositionFromPath( p, [ 1 ] ),
+						writer.createPositionFromPath( p, [ 5 ] )
+					),
+					usingOperation: false
+				} );
+
+				writer.setSelection( writer.createPositionFromPath( p, [ 3 ] ) );
+			} );
+		} );
+
+		it( 'should be up to date when post fixers have changed the data', () => {
+			model.schema.extend( '$text', { allowAttributes: 'foo' } );
+
+			const p = doc.getRoot().getChild( 0 );
+
+			doc.registerPostFixer( writer => {
+				writer.setAttribute( 'foo', 'biz', p.getChild( 0 ) );
+				writer.removeMarker( 'marker-1' );
+				writer.addMarker( 'marker-2', {
+					range: writer.createRange(
+						writer.createPositionFromPath( p, [ 1 ] ),
+						writer.createPositionFromPath( p, [ 5 ] )
+					),
+					usingOperation: false
+				} );
+			} );
+
+			model.change( writer => {
+				writer.insertText( 'abcdef', { foo: 'bar' }, p );
+
+				writer.addMarker( 'marker-1', {
+					range: writer.createRange(
+						writer.createPositionFromPath( p, [ 1 ] ),
+						writer.createPositionFromPath( p, [ 5 ] )
+					),
+					usingOperation: false
+				} );
+
+				writer.setSelection( writer.createPositionFromPath( p, [ 3 ] ) );
+			} );
+
+			expect( model.document.selection.getAttribute( 'foo' ) ).to.equal( 'biz' );
+			expect( Array.from( model.document.selection.markers, m => m.name ) ).to.deep.equal( [ 'marker-2' ] );
+		} );
+	} );
+
 	// DocumentSelection uses LiveRanges so here are only simple test to see if integration is
 	// DocumentSelection uses LiveRanges so here are only simple test to see if integration is
 	// working well, without getting into complicated corner cases.
 	// working well, without getting into complicated corner cases.
 	describe( 'after applying an operation should get updated and fire events', () => {
 	describe( 'after applying an operation should get updated and fire events', () => {

+ 38 - 16
packages/ckeditor5-engine/tests/model/selection.js

@@ -1117,6 +1117,20 @@ describe( 'Selection', () => {
 			);
 			);
 		} );
 		} );
 
 
+		it( 'does not go beyond limit elements', () => {
+			model.schema.register( 'table', { isBlock: true, isLimit: true, isObject: true, allowIn: '$root' } );
+			model.schema.register( 'tableRow', { allowIn: 'table', isLimit: true } );
+			model.schema.register( 'tableCell', { allowIn: 'tableRow', isObject: true } );
+
+			model.schema.register( 'blk', { allowIn: [ '$root', 'tableCell' ], isObject: true, isBlock: true } );
+
+			model.schema.extend( 'p', { allowIn: 'tableCell' } );
+
+			setData( model, '<table><tableRow><tableCell><p>foo</p>[<blk></blk><p>bar]</p></tableCell></tableRow></table>' );
+
+			expect( stringifyBlocks( doc.selection.getTopMostBlocks() ) ).to.deep.equal( [ 'blk', 'p#bar' ] );
+		} );
+
 		// Map all elements to data of its first child text node.
 		// Map all elements to data of its first child text node.
 		function toText( elements ) {
 		function toText( elements ) {
 			return Array.from( elements ).map( el => {
 			return Array.from( elements ).map( el => {
@@ -1128,11 +1142,11 @@ describe( 'Selection', () => {
 	describe( 'getTopMostBlocks()', () => {
 	describe( 'getTopMostBlocks()', () => {
 		beforeEach( () => {
 		beforeEach( () => {
 			model.schema.register( 'p', { inheritAllFrom: '$block' } );
 			model.schema.register( 'p', { inheritAllFrom: '$block' } );
-			model.schema.register( 'lvl0', { isBlock: true, isLimit: true, isObject: true, allowIn: '$root' } );
-			model.schema.register( 'lvl1', { allowIn: 'lvl0', isLimit: true } );
-			model.schema.register( 'lvl2', { allowIn: 'lvl1', isObject: true } );
+			model.schema.register( 'table', { isBlock: true, isLimit: true, isObject: true, allowIn: '$root' } );
+			model.schema.register( 'tableRow', { allowIn: 'table', isLimit: true } );
+			model.schema.register( 'tableCell', { allowIn: 'tableRow', isObject: true } );
 
 
-			model.schema.extend( 'p', { allowIn: 'lvl2' } );
+			model.schema.extend( 'p', { allowIn: 'tableCell' } );
 		} );
 		} );
 
 
 		it( 'returns an iterator', () => {
 		it( 'returns an iterator', () => {
@@ -1169,28 +1183,24 @@ describe( 'Selection', () => {
 		} );
 		} );
 
 
 		it( 'returns only top most blocks', () => {
 		it( 'returns only top most blocks', () => {
-			setData( model, '[<p>foo</p><lvl0><lvl1><lvl2><p>bar</p></lvl2></lvl1></lvl0><p>baz</p>]' );
+			setData( model, '[<p>foo</p><table><tableRow><tableCell><p>bar</p></tableCell></tableRow></table><p>baz</p>]' );
 
 
-			expect( stringifyBlocks( doc.selection.getTopMostBlocks() ) ).to.deep.equal( [ 'p#foo', 'lvl0', 'p#baz' ] );
+			expect( stringifyBlocks( doc.selection.getTopMostBlocks() ) ).to.deep.equal( [ 'p#foo', 'table', 'p#baz' ] );
 		} );
 		} );
 
 
 		it( 'returns only selected blocks even if nested in other blocks', () => {
 		it( 'returns only selected blocks even if nested in other blocks', () => {
-			setData( model, '<p>foo</p><lvl0><lvl1><lvl2><p>[b]ar</p></lvl2></lvl1></lvl0><p>baz</p>' );
+			setData( model, '<p>foo</p><table><tableRow><tableCell><p>[b]ar</p></tableCell></tableRow></table><p>baz</p>' );
 
 
 			expect( stringifyBlocks( doc.selection.getTopMostBlocks() ) ).to.deep.equal( [ 'p#bar' ] );
 			expect( stringifyBlocks( doc.selection.getTopMostBlocks() ) ).to.deep.equal( [ 'p#bar' ] );
 		} );
 		} );
 
 
-		// Map all elements to names. If element contains child text node it will be appended to name with '#'.
-		function stringifyBlocks( elements ) {
-			return Array.from( elements ).map( el => {
-				const name = el.name;
+		it( 'returns only selected blocks even if nested in other blocks (selection on the block)', () => {
+			model.schema.register( 'blk', { allowIn: [ '$root', 'tableCell' ], isObject: true, isBlock: true } );
 
 
-				const firstChild = el.getChild( 0 );
-				const hasText = firstChild && firstChild.data;
+			setData( model, '<table><tableRow><tableCell><p>foo</p>[<blk></blk><p>bar]</p></tableCell></tableRow></table>' );
 
 
-				return hasText ? `${ name }#${ firstChild.data }` : name;
-			} );
-		}
+			expect( stringifyBlocks( doc.selection.getTopMostBlocks() ) ).to.deep.equal( [ 'blk', 'p#bar' ] );
+		} );
 	} );
 	} );
 
 
 	describe( 'attributes interface', () => {
 	describe( 'attributes interface', () => {
@@ -1366,4 +1376,16 @@ describe( 'Selection', () => {
 			expect( doc.selection.containsEntireContent() ).to.equal( false );
 			expect( doc.selection.containsEntireContent() ).to.equal( false );
 		} );
 		} );
 	} );
 	} );
+
+	// Map all elements to names. If element contains child text node it will be appended to name with '#'.
+	function stringifyBlocks( elements ) {
+		return Array.from( elements ).map( el => {
+			const name = el.name;
+
+			const firstChild = el.getChild( 0 );
+			const hasText = firstChild && firstChild.data;
+
+			return hasText ? `${ name }#${ firstChild.data }` : name;
+		} );
+	}
 } );
 } );