Sfoglia il codice sorgente

Merge branch 't/1460' into t/ot-followups

Szymon Cofalik 7 anni fa
parent
commit
fa2978b42a

+ 59 - 17
packages/ckeditor5-engine/src/model/schema.js

@@ -668,35 +668,60 @@ export default class Schema {
 	 * @returns {Array.<module:engine/model/range~Range>} Ranges in which the attribute is allowed.
 	 */
 	getValidRanges( ranges, attribute ) {
-		const validRanges = [];
+		ranges = convertToMinimalFlatRanges( ranges );
+		const result = [];
 
 		for ( const range of ranges ) {
-			let last = range.start;
-			let from = range.start;
-			const to = range.end;
-
-			for ( const value of range.getWalker() ) {
-				if ( !this.checkAttribute( value.item, attribute ) ) {
-					if ( !from.isEqual( last ) ) {
-						validRanges.push( new Range( from, last ) );
-					}
+			const validRanges = this._getValidRangesForRange( range, attribute );
 
-					from = value.nextPosition;
-				}
+			result.push( ...validRanges );
+		}
+
+		return result;
+	}
+
+	/**
+	 * Takes a flat range and an attribute name. Traverses the range recursively and deeply to find and return all ranges
+	 * inside the given range on which the attribute can be applied.
+	 *
+	 * This is a helper function for {@link ~Schema#getValidRanges}.
+	 *
+	 * @private
+	 * @param {module:engine/model/range~Range} range Range to process.
+	 * @param {String} attribute The name of the attribute to check.
+	 * @returns {Array.<module:engine/model/range~Range>} Ranges in which the attribute is allowed.
+	 */
+	_getValidRangesForRange( range, attribute ) {
+		const result = [];
 
-				last = value.nextPosition;
+		let start = range.start;
+		let end = range.start;
+
+		for ( const item of range.getItems( { shallow: true } ) ) {
+			if ( item.is( 'element' ) ) {
+				result.push( ...this._getValidRangesForRange( Range.createIn( item ), attribute ) );
 			}
 
-			if ( from && !from.isEqual( to ) ) {
-				validRanges.push( new Range( from, to ) );
+			if ( !this.checkAttribute( item, attribute ) ) {
+				if ( !start.isEqual( end ) ) {
+					result.push( new Range( start, end ) );
+				}
+
+				start = Position.createAfter( item );
 			}
+
+			end = Position.createAfter( item );
 		}
 
-		return validRanges;
+		if ( !start.isEqual( end ) ) {
+			result.push( new Range( start, end ) );
+		}
+
+		return result;
 	}
 
 	/**
-	 * Basing on given the `position`, finds and returns a {@link module:engine/model/range~Range range} which is
+	 * Basing on given `position`, finds and returns a {@link module:engine/model/range~Range range} which is
 	 * nearest to that `position` and is a correct range for selection.
 	 *
 	 * The correct selection range might be collapsed when it is located in a position where the text node can be placed.
@@ -1572,3 +1597,20 @@ function* combineWalkers( backward, forward ) {
 		}
 	}
 }
+
+// Takes an array of non-intersecting ranges. For each of them gets minimal flat ranges covering that range and returns
+// all those minimal flat ranges.
+//
+// @param {Array.<module:engine/model/range~Range>} ranges Ranges to process.
+// @returns {Array.<module:engine/model/range~Range>} Minimal flat ranges of given `ranges`.
+function convertToMinimalFlatRanges( ranges ) {
+	const result = [];
+
+	for ( const range of ranges ) {
+		const minimal = range.getMinimalFlatRanges();
+
+		result.push( ...minimal );
+	}
+
+	return result;
+}

+ 102 - 70
packages/ckeditor5-engine/tests/model/schema.js

@@ -1106,8 +1106,7 @@ describe( 'Schema', () => {
 	} );
 
 	describe( 'getValidRanges()', () => {
-		const attribute = 'bold';
-		let model, doc, root, schema, ranges;
+		let model, doc, root, schema;
 
 		beforeEach( () => {
 			model = new Model();
@@ -1116,112 +1115,145 @@ describe( 'Schema', () => {
 			root = doc.createRoot();
 
 			schema.register( 'p', { inheritAllFrom: '$block' } );
-			schema.register( 'h1', { inheritAllFrom: '$block' } );
-			schema.register( 'img', {
-				allowWhere: '$text'
-			} );
+			schema.register( 'img', { allowWhere: '$text' } );
 
-			schema.addAttributeCheck( ( ctx, attributeName ) => {
-				// Allow 'bold' on p>$text.
-				if ( ctx.endsWith( 'p $text' ) && attributeName == 'bold' ) {
-					return true;
-				}
+			// This is a "hack" to allow setting any ranges in `setData` util.
+			schema.extend( '$text', { allowIn: '$root' } );
+		} );
 
-				// Allow 'bold' on $root>p.
-				if ( ctx.endsWith( '$root p' ) && attributeName == 'bold' ) {
-					return true;
-				}
-			} );
+		function test( input, attribute, output ) {
+			setData( model, input );
 
-			// Parse data string to model.
-			const parsedModel = parse( '<p>foo<img />bar</p>', model.schema, { context: [ root.name ] } );
+			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
+			const sel = new Selection( validRanges );
 
-			// Set parsed model data to prevent selection post-fixer from running.
-			model.change( writer => {
-				writer.insert( parsedModel, root );
-			} );
+			expect( stringify( root, sel ) ).to.equal( output );
+		}
 
-			ranges = [ Range.createOn( root.getChild( 0 ) ) ];
+		it( 'should return a range with p for an attribute allowed only on p', () => {
+			schema.extend( 'p', { allowAttributes: 'foo' } );
+
+			test(
+				'[<p>foo<img></img>bar</p>]',
+				'foo',
+				'[<p>foo<img></img>bar</p>]'
+			);
 		} );
 
-		it( 'should return unmodified ranges when attribute is allowed on each item (text is not allowed in img)', () => {
-			schema.extend( 'img', { allowAttributes: 'bold' } );
+		it( 'should return ranges on text nodes for an attribute allowed only on text', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 
-			expect( schema.getValidRanges( ranges, attribute ) ).to.deep.equal( ranges );
+			test(
+				'[<p>foo<img></img>bar</p>]',
+				'bold',
+				'<p>[foo]<img></img>[bar]</p>'
+			);
 		} );
 
-		it( 'should return unmodified ranges when attribute is allowed on each item (text is allowed in img)', () => {
-			schema.extend( 'img', { allowAttributes: 'bold' } );
-			schema.extend( '$text', { allowIn: 'img' } );
+		it( 'should return a range on img for an attribute allowed only on img', () => {
+			schema.extend( 'img', { allowAttributes: 'src' } );
 
-			expect( schema.getValidRanges( ranges, attribute ) ).to.deep.equal( ranges );
+			test(
+				'[<p>foo<img></img>bar</p>]',
+				'src',
+				'<p>foo[<img></img>]bar</p>'
+			);
 		} );
 
-		it( 'should return two ranges when attribute is not allowed on one item', () => {
+		it( 'should return a range containing all children for an attribute allowed on all children', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 			schema.extend( 'img', { allowAttributes: 'bold' } );
-			schema.extend( '$text', { allowIn: 'img' } );
 
-			setData( model, '<p>[foo<img>xxx</img>bar]</p>' );
+			test(
+				'[<p>foo<img></img>bar</p>]',
+				'bold',
+				'<p>[foo<img></img>bar]</p>'
+			);
+		} );
 
-			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
-			const sel = new Selection( validRanges );
+		it( 'should return a range with p and a range on all children for an attribute allowed on p and its children', () => {
+			schema.extend( 'p', { allowAttributes: 'foo' } );
+			schema.extend( '$text', { allowAttributes: 'foo' } );
+			schema.extend( 'img', { allowAttributes: 'foo' } );
 
-			expect( stringify( root, sel ) ).to.equal( '<p>[foo<img>]xxx[</img>bar]</p>' );
-		} );
+			setData( model, '[<p>foo<img></img>bar</p>]' );
 
-		it( 'should return three ranges when attribute is not allowed on one element but is allowed on its child', () => {
-			schema.extend( '$text', { allowIn: 'img' } );
+			const validRanges = schema.getValidRanges( doc.selection.getRanges(), 'foo' );
 
-			schema.addAttributeCheck( ( ctx, attributeName ) => {
-				// Allow 'bold' on img>$text.
-				if ( ctx.endsWith( 'img $text' ) && attributeName == 'bold' ) {
-					return true;
-				}
-			} );
+			expect( validRanges.length ).to.equal( 2 );
 
-			setData( model, '<p>[foo<img>xxx</img>bar]</p>' );
+			expect( validRanges[ 0 ].start.path ).to.deep.equal( [ 0, 0 ] );
+			expect( validRanges[ 0 ].end.path ).to.deep.equal( [ 0, 7 ] );
 
-			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
-			const sel = new Selection( validRanges );
+			expect( validRanges[ 1 ].start.path ).to.deep.equal( [ 0 ] );
+			expect( validRanges[ 1 ].end.path ).to.deep.equal( [ 1 ] );
+		} );
+
+		it( 'should not break a range if children are not allowed to have the attribute', () => {
+			schema.extend( 'p', { allowAttributes: 'foo' } );
 
-			expect( stringify( root, sel ) ).to.equal( '<p>[foo]<img>[xxx]</img>[bar]</p>' );
+			test(
+				'[<p>foo</p><p>bar</p>]',
+				'foo',
+				'[<p>foo</p><p>bar</p>]'
+			);
 		} );
 
-		it( 'should not leak beyond the given ranges', () => {
-			setData( model, '<p>[foo<img></img>bar]x[bar<img></img>foo]</p>' );
+		it( 'should search deeply', () => {
+			schema.extend( '$text', { allowAttributes: 'bold', allowIn: 'img' } );
 
-			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
-			const sel = new Selection( validRanges );
+			test(
+				'[<p>foo<img>xxx</img>bar</p>]',
+				'bold',
+				'<p>[foo]<img>[xxx]</img>[bar]</p>'
+			);
+		} );
+
+		it( 'should work with multiple ranges', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 
-			expect( stringify( root, sel ) ).to.equal( '<p>[foo]<img></img>[bar]x[bar]<img></img>[foo]</p>' );
+			test(
+				'[<p>a</p><p>b</p>]<p>c</p><p>[d]</p>',
+				'bold',
+				'<p>[a]</p><p>[b]</p><p>c</p><p>[d]</p>'
+			);
 		} );
 
-		it( 'should correctly handle a range which ends in a disallowed position', () => {
-			schema.extend( '$text', { allowIn: 'img' } );
+		it( 'should work with non-flat ranges', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 
-			setData( model, '<p>[foo<img>bar]</img>bom</p>' );
+			test(
+				'[<p>a</p><p>b</p><p>c]</p><p>d</p>',
+				'bold',
+				'<p>[a]</p><p>[b]</p><p>[c]</p><p>d</p>'
+			);
+		} );
 
-			const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute );
-			const sel = new Selection( validRanges );
+		it( 'should not leak beyond the given ranges', () => {
+			schema.extend( '$text', { allowAttributes: 'bold' } );
 
-			expect( stringify( root, sel ) ).to.equal( '<p>[foo]<img>bar</img>bom</p>' );
+			test(
+				'[<p>foo</p><p>b]a[r</p><p>x]yz</p>',
+				'bold',
+				'<p>[foo]</p><p>[b]a[r]</p><p>[x]yz</p>'
+			);
 		} );
 
-		it( 'should split range into two ranges and omit disallowed element', () => {
+		it( 'should correctly handle a range which ends in a disallowed position', () => {
+			schema.extend( '$text', { allowAttributes: 'bold', allowIn: 'img' } );
+
+			// Disallow bold on text inside image.
 			schema.addAttributeCheck( ( ctx, attributeName ) => {
-				// Disallow 'bold' on p>img.
-				if ( ctx.endsWith( 'p img' ) && attributeName == 'bold' ) {
+				if ( ctx.endsWith( 'img $text' ) && attributeName == 'bold' ) {
 					return false;
 				}
 			} );
 
-			const result = schema.getValidRanges( ranges, attribute );
-
-			expect( result ).to.length( 2 );
-			expect( result[ 0 ].start.path ).to.members( [ 0 ] );
-			expect( result[ 0 ].end.path ).to.members( [ 0, 3 ] );
-			expect( result[ 1 ].start.path ).to.members( [ 0, 4 ] );
-			expect( result[ 1 ].end.path ).to.members( [ 1 ] );
+			test(
+				'[<p>foo<img>xx]x</img>bar</p>',
+				'bold',
+				'<p>[foo]<img>xxx</img>bar</p>'
+			);
 		} );
 	} );