Преглед на файлове

Added: Introduced `strict` parameter in model.Range#containsRange and view.Range#containsRange.

Szymon Cofalik преди 8 години
родител
ревизия
bd6976680a

+ 1 - 1
packages/ckeditor5-engine/src/conversion/modelconversiondispatcher.js

@@ -224,7 +224,7 @@ export default class ModelConversionDispatcher {
 			}
 
 			// Check if inserted content contains a marker.
-			if ( range.containsRange( markerRange ) || range.isEqual( markerRange ) ) {
+			if ( range.containsRange( markerRange, true ) ) {
 				this.convertMarker( 'addMarker', marker.name, markerRange );
 			}
 		}

+ 17 - 8
packages/ckeditor5-engine/src/model/range.js

@@ -90,8 +90,8 @@ export default class Range {
 	 * Checks whether this range contains given {@link module:engine/model/position~Position position}.
 	 *
 	 * @param {module:engine/model/position~Position} position Position to check.
-	 * @returns {Boolean} `true` if given {@link module:engine/model/position~Position position} is contained in this range,
-	 * `false` otherwise.
+	 * @returns {Boolean} `true` if given {@link module:engine/model/position~Position position} is contained
+	 * in this range,`false` otherwise.
 	 */
 	containsPosition( position ) {
 		return position.isAfter( this.start ) && position.isBefore( this.end );
@@ -101,15 +101,24 @@ export default class Range {
 	 * Checks whether this range contains given {@link ~Range range}.
 	 *
 	 * @param {module:engine/model/range~Range} otherRange Range to check.
+	 * @param {Boolean} [strict=true] Whether the check is strict or loose. If the check is strict (`true`), compared range cannot
+	 * start or end at the same position as this range boundaries. If the check is loose (`false`), compared range can start, end or
+	 * even be equal to this range. Note that collapsed ranges are always compared in strict mode.
 	 * @returns {Boolean} `true` if given {@link ~Range range} boundaries are contained by this range, `false` otherwise.
 	 */
-	containsRange( otherRange ) {
-		return this.containsPosition( otherRange.start ) && this.containsPosition( otherRange.end );
+	containsRange( otherRange, strict = true ) {
+		if ( otherRange.isCollapsed ) {
+			strict = true;
+		}
+
+		const containsStart = this.containsPosition( otherRange.start ) || ( !strict && this.start.isEqual( otherRange.start ) );
+		const containsEnd = this.containsPosition( otherRange.end ) || ( !strict && this.end.isEqual( otherRange.end ) );
+
+		return containsStart && containsEnd;
 	}
 
 	/**
-	 * Two ranges are equal if their {@link #start start} and
-	 * {@link #end end} positions are equal.
+	 * Two ranges are equal if their {@link #start start} and {@link #end end} positions are equal.
 	 *
 	 * @param {module:engine/model/range~Range} otherRange Range to compare with.
 	 * @returns {Boolean} `true` if ranges are equal, `false` otherwise.
@@ -418,8 +427,8 @@ export default class Range {
 		}
 
 		// It may happen that a range is split into two, and then the part of second "piece" is moved into first
-		// "piece". In this case we will have incorrect third rage, which should not be included in the result --
-		// because it is already included in first "piece". In this loop we are looking for all such ranges that
+		// "piece". In this case we will have incorrect third range, which should not be included in the result --
+		// because it is already included in the first "piece". In this loop we are looking for all such ranges that
 		// are inside other ranges and we simply remove them.
 		for ( let i = 0; i < ranges.length; i++ ) {
 			const range = ranges[ i ];

+ 12 - 2
packages/ckeditor5-engine/src/view/range.js

@@ -172,11 +172,21 @@ export default class Range {
 	 * Checks whether this range contains given {@link module:engine/view/range~Range range}.
 	 *
 	 * @param {module:engine/view/range~Range} otherRange Range to check.
+	 * @param {Boolean} [strict=true] Whether the check is strict or loose. If the check is strict (`true`), compared range cannot
+	 * start or end at the same position as this range boundaries. If the check is loose (`false`), compared range can start, end or
+	 * even be equal to this range. Note that collapsed ranges are always compared in strict mode.
 	 * @returns {Boolean} `true` if given {@link module:engine/view/range~Range range} boundaries are contained by this range, `false`
 	 * otherwise.
 	 */
-	containsRange( otherRange ) {
-		return this.containsPosition( otherRange.start ) && this.containsPosition( otherRange.end );
+	containsRange( otherRange, strict = true ) {
+		if ( otherRange.isCollapsed ) {
+			strict = true;
+		}
+
+		const containsStart = this.containsPosition( otherRange.start ) || ( !strict && this.start.isEqual( otherRange.start ) );
+		const containsEnd = this.containsPosition( otherRange.end ) || ( !strict && this.end.isEqual( otherRange.end ) );
+
+		return containsStart && containsEnd;
 	}
 
 	/**

+ 37 - 10
packages/ckeditor5-engine/tests/controller/editingcontroller.js

@@ -289,22 +289,49 @@ describe( 'EditingController', () => {
 			editing.modelToView.convertMarker.restore();
 		} );
 
-		it( 'should forward add marker event if inserted content has a marker (inserted element from outside of tree)', () => {
-			const element = new ModelElement( new ModelText( 'foo' ) );
-			modelRoot.appendChildren( element );
+		describe( 'should forward add marker event if inserted content has a marker', () => {
+			let element, outerRange;
 
-			const markerRange = ModelRange.createFromParentsAndOffsets( element, 1, element, 2 );
-			const outerRange = ModelRange.createOn( element );
+			beforeEach( () => {
+				element = new ModelElement( new ModelText( 'foo' ) );
+				modelRoot.appendChildren( element );
 
-			model.markers.set( 'name', markerRange );
+				outerRange = ModelRange.createOn( element );
 
-			sinon.spy( editing.modelToView, 'convertMarker' );
+				sinon.spy( editing.modelToView, 'convertMarker' );
+			} );
 
-			editing.modelToView.convertInsertion( outerRange );
+			afterEach( () => {
+				editing.modelToView.convertMarker.restore();
+			} );
 
-			expect( editing.modelToView.convertMarker.calledWithExactly( 'addMarker', 'name', markerRange ) ).to.be.true;
+			it( 'marker strictly contained', () => {
+				const markerRange = ModelRange.createFromParentsAndOffsets( element, 1, element, 2 );
+				model.markers.set( 'name', markerRange );
+				editing.modelToView.convertInsertion( outerRange );
+				expect( editing.modelToView.convertMarker.calledWithExactly( 'addMarker', 'name', markerRange ) ).to.be.true;
+			} );
 
-			editing.modelToView.convertMarker.restore();
+			it( 'marker starts at same position', () => {
+				const markerRange = ModelRange.createFromParentsAndOffsets( element, 0, element, 2 );
+				model.markers.set( 'name', markerRange );
+				editing.modelToView.convertInsertion( outerRange );
+				expect( editing.modelToView.convertMarker.calledWithExactly( 'addMarker', 'name', markerRange ) ).to.be.true;
+			} );
+
+			it( 'marker ends at same position', () => {
+				const markerRange = ModelRange.createFromParentsAndOffsets( element, 1, element, 3 );
+				model.markers.set( 'name', markerRange );
+				editing.modelToView.convertInsertion( outerRange );
+				expect( editing.modelToView.convertMarker.calledWithExactly( 'addMarker', 'name', markerRange ) ).to.be.true;
+			} );
+
+			it( 'marker is same as range', () => {
+				const markerRange = ModelRange.createFromParentsAndOffsets( element, 0, element, 3 );
+				model.markers.set( 'name', markerRange );
+				editing.modelToView.convertInsertion( outerRange );
+				expect( editing.modelToView.convertMarker.calledWithExactly( 'addMarker', 'name', markerRange ) ).to.be.true;
+			} );
 		} );
 
 		it( 'should not start marker conversion if content is not inserted into any marker range', () => {

+ 0 - 3
packages/ckeditor5-engine/tests/conversion/modelconversiondispatcher.js

@@ -391,9 +391,6 @@ describe( 'ModelConversionDispatcher', () => {
 		} );
 	} );
 
-	describe( 'convertRename', () => {
-	} );
-
 	describe( 'convertAttribute', () => {
 		it( 'should fire event for every item in passed range', () => {
 			root.appendChildren( [

+ 26 - 0
packages/ckeditor5-engine/tests/model/range.js

@@ -390,6 +390,32 @@ describe( 'Range', () => {
 
 			expect( range.containsPosition( position ) ).to.be.true;
 		} );
+
+		it( 'should return true if ranges are equal and check is not strict', () => {
+			const otherRange = Range.createFromRange( range );
+
+			expect( range.containsRange( otherRange, false ) ).to.be.true;
+		} );
+
+		it( 'should return true if ranges start at the same position and check is not strict', () => {
+			const otherRange = new Range( range.start, new Position( root, [ 2 ] ) );
+
+			expect( range.containsRange( otherRange, false ) ).to.be.true;
+		} );
+
+		it( 'should return true if ranges end at the same position and check is not strict', () => {
+			const otherRange = new Range( new Position( root, [ 2 ] ), range.end );
+
+			expect( range.containsRange( otherRange, false ) ).to.be.true;
+		} );
+
+		it( 'should return false if given range is collapsed and starts or ends at another range boundary', () => {
+			expect( range.containsRange( new Range( range.start, range.start ) ) ).to.be.false;
+			expect( range.containsRange( new Range( range.end, range.end ) ) ).to.be.false;
+
+			expect( range.containsRange( new Range( range.start, range.start ), false ) ).to.be.false;
+			expect( range.containsRange( new Range( range.end, range.end ), false ) ).to.be.false;
+		} );
 	} );
 
 	describe( '_getTransformedByInsertion', () => {

+ 26 - 0
packages/ckeditor5-engine/tests/view/range.js

@@ -301,6 +301,32 @@ describe( 'Range', () => {
 
 			expect( range.containsRange( otherRange ) ).to.be.true;
 		} );
+
+		it( 'should return true if ranges are equal and check is not strict', () => {
+			const otherRange = Range.createFromRange( range );
+
+			expect( range.containsRange( otherRange, false ) ).to.be.true;
+		} );
+
+		it( 'should return true if ranges start at the same position and check is not strict', () => {
+			const otherRange = new Range( range.start, afterX );
+
+			expect( range.containsRange( otherRange, false ) ).to.be.true;
+		} );
+
+		it( 'should return true if ranges end at the same position and check is not strict', () => {
+			const otherRange = new Range( beforeB, range.end );
+
+			expect( range.containsRange( otherRange, false ) ).to.be.true;
+		} );
+
+		it( 'should return false if given range is collapsed and starts or ends at another range boundary', () => {
+			expect( range.containsRange( new Range( range.start, range.start ) ) ).to.be.false;
+			expect( range.containsRange( new Range( range.end, range.end ) ) ).to.be.false;
+
+			expect( range.containsRange( new Range( range.start, range.start ), false ) ).to.be.false;
+			expect( range.containsRange( new Range( range.end, range.end ), false ) ).to.be.false;
+		} );
 	} );
 
 	describe( 'other range interaction', () => {