Explorar o código

Make Position.createAt() offset parameter required.

Maciej Gołaszewski %!s(int64=7) %!d(string=hai) anos
pai
achega
90d2a88bf7

+ 2 - 2
packages/ckeditor5-engine/src/controller/datacontroller.js

@@ -201,7 +201,7 @@ export default class DataController {
 		const modelRoot = this.model.document.getRoot( rootName );
 
 		this.model.enqueueChange( 'transparent', writer => {
-			writer.insert( this.parse( data, modelRoot ), modelRoot );
+			writer.insert( this.parse( data, modelRoot ), modelRoot, 0 );
 		} );
 
 		return Promise.resolve();
@@ -228,7 +228,7 @@ export default class DataController {
 			writer.removeSelectionAttribute( this.model.document.selection.getAttributeKeys() );
 
 			writer.remove( ModelRange.createIn( modelRoot ) );
-			writer.insert( this.parse( data, modelRoot ), modelRoot );
+			writer.insert( this.parse( data, modelRoot ), modelRoot, 0 );
 		} );
 	}
 

+ 2 - 2
packages/ckeditor5-engine/src/conversion/upcast-converters.js

@@ -358,7 +358,7 @@ function _prepareToElementConverter( config ) {
 		conversionApi.writer.insert( modelElement, splitResult.position );
 
 		// Convert children and insert to element.
-		const childrenResult = conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( modelElement ) );
+		const childrenResult = conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( modelElement, 0 ) );
 
 		// Consume appropriate value from consumable values list.
 		conversionApi.consumable.consume( data.viewItem, match.match );
@@ -380,7 +380,7 @@ function _prepareToElementConverter( config ) {
 		// before: <allowed><notAllowed>[]</notAllowed></allowed>
 		// after:  <allowed><notAllowed></notAllowed><converted></converted><notAllowed>[]</notAllowed></allowed>
 		if ( splitResult.cursorParent ) {
-			data.modelCursor = ModelPosition.createAt( splitResult.cursorParent );
+			data.modelCursor = ModelPosition.createAt( splitResult.cursorParent, 0 );
 
 			// Otherwise just continue after inserted element.
 		} else {

+ 2 - 2
packages/ckeditor5-engine/src/conversion/upcastdispatcher.js

@@ -71,7 +71,7 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix';
  *  	 				conversionApi.writer.insert( paragraph, splitResult.position );
  *
  *  	 				// Convert children to paragraph.
- *  	 				const { modelRange } = conversionApi.convertChildren( data.viewItem, Position.createAt( paragraph ) );
+ *  	 				const { modelRange } = conversionApi.convertChildren( data.viewItem, Position.createAt( paragraph, 0 ) );
  *
  * 						// Set as conversion result, attribute converters may use this property.
  *  	 				data.modelRange = new Range( Position.createBefore( paragraph ), modelRange.end );
@@ -419,7 +419,7 @@ function createContextTree( contextDefinition, writer ) {
 			writer.append( current, position );
 		}
 
-		position = ModelPosition.createAt( current );
+		position = ModelPosition.createAt( current, 0 );
 	}
 
 	return position;

+ 1 - 1
packages/ckeditor5-engine/src/dev-utils/model.js

@@ -391,7 +391,7 @@ function convertToModelElement() {
 
 		conversionApi.mapper.bindElements( element, data.viewItem );
 
-		conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( element ) );
+		conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( element, 0 ) );
 
 		data.modelRange = ModelRange.createOn( element );
 		data.modelCursor = data.modelRange.end;

+ 5 - 3
packages/ckeditor5-engine/src/model/position.js

@@ -816,7 +816,7 @@ export default class Position {
 	 * * {@link module:engine/model/position~Position.createFromPosition}.
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/position~Position} itemOrPosition
-	 * @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
+	 * @param {Number|'end'|'before'|'after'} offset Offset or one of the flags. Used only when
 	 * first parameter is a {@link module:engine/model/item~Item model item}.
 	 */
 	static createAt( itemOrPosition, offset ) {
@@ -831,8 +831,10 @@ export default class Position {
 				return this.createBefore( node );
 			} else if ( offset == 'after' ) {
 				return this.createAfter( node );
-			} else if ( !offset ) {
-				offset = 0;
+			} else if ( offset !== 0 && !offset ) {
+				throw new CKEditorError(
+					'model-position-createAt-required-second-parameter: ' +
+					'Position.createAt requires the second parameter offset.' );
 			}
 
 			return this.createFromParentAndOffset( node, offset );

+ 1 - 1
packages/ckeditor5-engine/src/model/schema.js

@@ -1162,7 +1162,7 @@ export class SchemaContext {
  *		schema.checkChild( contextDefinition, childToCheck );
  *
  *		// Also check in [ rootElement, blockQuoteElement, paragraphElement ].
- *		schema.checkChild( Position.createAt( paragraphElement ), 'foo' );
+ *		schema.checkChild( Position.createAt( paragraphElement, 0 ), 'foo' );
  *
  *		// Check in [ rootElement, paragraphElement ].
  *		schema.checkChild( [ rootElement, paragraphElement ], 'foo' );

+ 2 - 2
packages/ckeditor5-engine/src/model/selection.js

@@ -665,7 +665,7 @@ export default class Selection {
 			const endBlock = getParentBlock( range.end, visited );
 
 			// #984. Don't return the end block if the range ends right at its beginning.
-			if ( endBlock && !range.end.isTouching( Position.createAt( endBlock ) ) ) {
+			if ( endBlock && !range.end.isTouching( Position.createAt( endBlock, 0 ) ) ) {
 				yield endBlock;
 			}
 		}
@@ -683,7 +683,7 @@ export default class Selection {
 	 * @returns {Boolean}
 	 */
 	containsEntireContent( element = this.anchor.root ) {
-		const limitStartPosition = Position.createAt( element );
+		const limitStartPosition = Position.createAt( element, 0 );
 		const limitEndPosition = Position.createAt( element, 'end' );
 
 		return limitStartPosition.isTouching( this.getFirstPosition() ) &&

+ 1 - 1
packages/ckeditor5-engine/src/model/utils/deletecontent.js

@@ -210,7 +210,7 @@ function replaceEntireContentWithParagraph( writer, selection ) {
 	const limitElement = writer.model.schema.getLimitElement( selection );
 
 	writer.remove( Range.createIn( limitElement ) );
-	insertParagraph( writer, Position.createAt( limitElement ), selection );
+	insertParagraph( writer, Position.createAt( limitElement, 0 ), selection );
 }
 
 // We want to replace the entire content with a paragraph when:

+ 1 - 1
packages/ckeditor5-engine/src/model/utils/getselectedcontent.js

@@ -99,7 +99,7 @@ export default function getSelectedContent( model, selection ) {
 			// Find the position of the original range in the cloned fragment.
 			const newRange = range._getTransformedByMove( flatSubtreeRange.start, Position.createAt( frag, 0 ), howMany )[ 0 ];
 
-			const leftExcessRange = new Range( Position.createAt( frag ), newRange.start );
+			const leftExcessRange = new Range( Position.createAt( frag, 0 ), newRange.start );
 			const rightExcessRange = new Range( newRange.end, Position.createAt( frag, 'end' ) );
 
 			removeRangeContent( rightExcessRange, writer );

+ 5 - 2
packages/ckeditor5-engine/src/model/utils/selection-post-fixer.js

@@ -199,8 +199,11 @@ function tryFixingNonCollapsedRage( range, schema ) {
 	if ( isStartInLimit || isEndInLimit ) {
 		// Although we've already found limit element on start/end positions we must find the outer-most limit element.
 		// as limit elements might be nested directly inside (ie table > tableRow > tableCell).
-		const fixedStart = isStartInLimit ? expandSelectionOnIsLimitNode( Position.createAt( startLimitElement ), schema, 'start' ) : start;
-		const fixedEnd = isEndInLimit ? expandSelectionOnIsLimitNode( Position.createAt( endLimitElement ), schema, 'end' ) : end;
+		const startPosition = Position.createAt( startLimitElement, 0 );
+		const endPosition = Position.createAt( endLimitElement, 0 );
+
+		const fixedStart = isStartInLimit ? expandSelectionOnIsLimitNode( startPosition, schema, 'start' ) : start;
+		const fixedEnd = isEndInLimit ? expandSelectionOnIsLimitNode( endPosition, schema, 'end' ) : end;
 
 		return new Range( fixedStart, fixedEnd );
 	}

+ 4 - 4
packages/ckeditor5-engine/src/model/writer.js

@@ -156,7 +156,7 @@ export default class Writer {
 	 * @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
 	 * second parameter is a {@link module:engine/model/item~Item model item}.
 	 */
-	insert( item, itemOrPosition, offset ) {
+	insert( item, itemOrPosition, offset = 0 ) {
 		this._assertWriterUsedCorrectly();
 
 		const position = Position.createAt( itemOrPosition, offset );
@@ -198,7 +198,7 @@ export default class Writer {
 		if ( item instanceof DocumentFragment ) {
 			for ( const [ markerName, markerRange ] of item.markers ) {
 				// We need to migrate marker range from DocumentFragment to Document.
-				const rangeRootPosition = Position.createAt( markerRange.root );
+				const rangeRootPosition = Position.createAt( markerRange.root, 0 );
 				const range = new Range(
 					markerRange.start._getCombined( rangeRootPosition, position ),
 					markerRange.end._getCombined( rangeRootPosition, position )
@@ -668,7 +668,7 @@ export default class Writer {
 
 		return {
 			position,
-			range: new Range( Position.createAt( firstSplitElement, 'end' ), Position.createAt( firstCopyElement ) )
+			range: new Range( Position.createAt( firstSplitElement, 'end' ), Position.createAt( firstCopyElement, 0 ) )
 		};
 	}
 
@@ -1043,7 +1043,7 @@ export default class Writer {
 	 * {@link module:engine/model/position~Position.createAt `Position.createAt()`} parameters.
 	 *
 	 * @param {module:engine/model/item~Item|module:engine/model/position~Position} itemOrPosition
-	 * @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
+	 * @param {Number|'end'|'before'|'after'} [of~fset=0] Offset or one of the flags. Used only when
 	 * first parameter is a {@link module:engine/model/item~Item model item}.
 	 */
 	setSelectionFocus( itemOrPosition, offset ) {

+ 1 - 1
packages/ckeditor5-engine/src/view/downcastwriter.js

@@ -915,7 +915,7 @@ export default class DowncastWriter {
 		const newElement = new ContainerElement( newName, viewElement.getAttributes() );
 
 		this.insert( Position.createAfter( viewElement ), newElement );
-		this.move( Range.createIn( viewElement ), Position.createAt( newElement ) );
+		this.move( Range.createIn( viewElement ), Position.createAt( newElement, 0 ) );
 		this.remove( Range.createOn( viewElement ) );
 
 		return newElement;

+ 2 - 2
packages/ckeditor5-engine/tests/conversion/upcast-converters.js

@@ -841,7 +841,7 @@ describe( 'upcast-converters', () => {
 					const paragraph = conversionApi.writer.createElement( 'paragraph' );
 
 					conversionApi.writer.insert( paragraph, data.modelCursor );
-					conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( paragraph ) );
+					conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( paragraph, 0 ) );
 
 					data.modelRange = ModelRange.createOn( paragraph );
 					data.modelCursor = data.modelRange.end;
@@ -863,7 +863,7 @@ describe( 'upcast-converters', () => {
 				new ViewContainerElement( 'div', null, [ new ViewText( 'abc' ), new ViewContainerElement( 'foo' ) ] ),
 				new ViewContainerElement( 'bar' )
 			] );
-			const position = ModelPosition.createAt( new ModelElement( 'element' ) );
+			const position = ModelPosition.createAt( new ModelElement( 'element' ), 0 );
 
 			dispatcher.on( 'documentFragment', convertToModelFragment() );
 			dispatcher.on( 'element', convertToModelFragment(), { priority: 'lowest' } );

+ 4 - 4
packages/ckeditor5-engine/tests/conversion/upcastdispatcher.js

@@ -501,7 +501,7 @@ describe( 'UpcastDispatcher', () => {
 				dispatcher.on( 'documentFragment', ( evt, data, conversionApi ) => {
 					spy();
 
-					const result = conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( rootMock ) );
+					const result = conversionApi.convertChildren( data.viewItem, ModelPosition.createAt( rootMock, 0 ) );
 
 					expect( result.modelRange ).to.be.instanceof( ModelRange );
 					expect( result.modelRange.start.path ).to.deep.equal( [ 0 ] );
@@ -540,7 +540,7 @@ describe( 'UpcastDispatcher', () => {
 				dispatcher.on( 'documentFragment', ( evt, data, conversionApi ) => {
 					const paragraph = conversionApi.writer.createElement( 'paragraph' );
 					const span = conversionApi.writer.createElement( 'span' );
-					const position = ModelPosition.createAt( paragraph );
+					const position = ModelPosition.createAt( paragraph, 0 );
 
 					const result = conversionApi.splitToAllowedParent( span, position );
 
@@ -572,7 +572,7 @@ describe( 'UpcastDispatcher', () => {
 					conversionApi.writer.insert( paragraph, section );
 					conversionApi.writer.insert( span, paragraph );
 
-					const position = ModelPosition.createAt( span );
+					const position = ModelPosition.createAt( span, 0 );
 
 					const paragraph2 = conversionApi.writer.createElement( 'paragraph' );
 					const result = conversionApi.splitToAllowedParent( paragraph2, position );
@@ -597,7 +597,7 @@ describe( 'UpcastDispatcher', () => {
 				dispatcher.on( 'documentFragment', ( evt, data, conversionApi ) => {
 					const paragraph = conversionApi.writer.createElement( 'paragraph' );
 					const span = conversionApi.writer.createElement( 'span' );
-					const position = ModelPosition.createAt( paragraph );
+					const position = ModelPosition.createAt( paragraph, 0 );
 
 					const result = conversionApi.splitToAllowedParent( span, position );
 

+ 7 - 3
packages/ckeditor5-engine/tests/model/position.js

@@ -158,17 +158,21 @@ describe( 'Position', () => {
 	} );
 
 	describe( 'createAt()', () => {
+		it( 'should throw if uknown offset is passed', () => {
+			expect( () => Position.createAt( ul ) ).to.throw( CKEditorError, /model-position-createAt-required-second-parameter/ );
+		} );
+
 		it( 'should create positions from positions', () => {
 			const spy = testUtils.sinon.spy( Position, 'createFromPosition' );
 
-			expect( Position.createAt( Position.createAt( ul ) ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0 ] );
+			expect( Position.createAt( Position.createAt( ul, 0 ) ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0 ] );
 
 			expect( spy.calledOnce ).to.be.true;
 		} );
 
 		it( 'should create positions from node and offset', () => {
-			expect( Position.createAt( ul ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0 ] );
-			expect( Position.createAt( li1 ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0, 0 ] );
+			expect( Position.createAt( ul, 0 ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0 ] );
+			expect( Position.createAt( li1, 0 ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0, 0 ] );
 			expect( Position.createAt( ul, 1 ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 1 ] );
 		} );
 

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

@@ -176,7 +176,7 @@ describe( 'Range', () => {
 		describe( 'createCollapsedAt()', () => {
 			it( 'should return new collapsed range at the given item position', () => {
 				const item = new Element( 'p', null, new Text( 'foo' ) );
-				const range = Range.createCollapsedAt( item );
+				const range = Range.createCollapsedAt( item, 0 );
 
 				expect( range.start.parent ).to.equal( item );
 				expect( range.start.offset ).to.equal( 0 );

+ 14 - 14
packages/ckeditor5-engine/tests/model/schema.js

@@ -394,8 +394,8 @@ describe( 'Schema', () => {
 		} );
 
 		it( 'accepts a schemaContext instance as a context', () => {
-			const rootContext = new SchemaContext( Position.createAt( root1 ) );
-			const paragraphContext = new SchemaContext( Position.createAt( r1p1 ) );
+			const rootContext = new SchemaContext( Position.createAt( root1, 0 ) );
+			const paragraphContext = new SchemaContext( Position.createAt( r1p1, 0 ) );
 
 			expect( schema.checkChild( rootContext, 'paragraph' ) ).to.be.true;
 			expect( schema.checkChild( rootContext, '$text' ) ).to.be.false;
@@ -405,8 +405,8 @@ describe( 'Schema', () => {
 		} );
 
 		it( 'accepts a position as a context', () => {
-			const posInRoot = Position.createAt( root1 );
-			const posInParagraph = Position.createAt( r1p1 );
+			const posInRoot = Position.createAt( root1, 0 );
+			const posInParagraph = Position.createAt( r1p1, 0 );
 
 			expect( schema.checkChild( posInRoot, 'paragraph' ) ).to.be.true;
 			expect( schema.checkChild( posInRoot, '$text' ) ).to.be.false;
@@ -486,16 +486,16 @@ describe( 'Schema', () => {
 		} );
 
 		it( 'accepts a position as a context', () => {
-			const posInRoot = Position.createAt( root1 );
-			const posInParagraph = Position.createAt( r1p1 );
+			const posInRoot = Position.createAt( root1, 0 );
+			const posInParagraph = Position.createAt( r1p1, 0 );
 
 			expect( schema.checkAttribute( posInRoot, 'align' ) ).to.be.false;
 			expect( schema.checkAttribute( posInParagraph, 'align' ) ).to.be.true;
 		} );
 
 		it( 'accepts a schemaContext instance as a context', () => {
-			const rootContext = new SchemaContext( Position.createAt( root1 ) );
-			const paragraphContext = new SchemaContext( Position.createAt( r1p1 ) );
+			const rootContext = new SchemaContext( Position.createAt( root1, 0 ) );
+			const paragraphContext = new SchemaContext( Position.createAt( r1p1, 0 ) );
 
 			expect( schema.checkAttribute( rootContext, 'align' ) ).to.be.false;
 			expect( schema.checkAttribute( paragraphContext, 'align' ) ).to.be.true;
@@ -1575,7 +1575,7 @@ describe( 'Schema', () => {
 		it( 'should return position ancestor that allows to insert given node to it', () => {
 			const node = new Element( 'paragraph' );
 
-			const allowedParent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
+			const allowedParent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );
 
 			expect( allowedParent ).to.equal( r1bQ );
 		} );
@@ -1583,7 +1583,7 @@ describe( 'Schema', () => {
 		it( 'should return position ancestor that allows to insert given node to it when position is already i such an element', () => {
 			const node = new Text( 'text' );
 
-			const parent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
+			const parent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );
 
 			expect( parent ).to.equal( r1bQp );
 		} );
@@ -1597,7 +1597,7 @@ describe( 'Schema', () => {
 			} );
 			const node = new Element( 'div' );
 
-			const parent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
+			const parent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );
 
 			expect( parent ).to.null;
 		} );
@@ -1611,7 +1611,7 @@ describe( 'Schema', () => {
 			} );
 			const node = new Element( 'div' );
 
-			const parent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
+			const parent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );
 
 			expect( parent ).to.null;
 		} );
@@ -1619,7 +1619,7 @@ describe( 'Schema', () => {
 		it( 'should return null when there is no allowed ancestor for given position', () => {
 			const node = new Element( 'section' );
 
-			const parent = schema.findAllowedParent( node, Position.createAt( r1bQp ) );
+			const parent = schema.findAllowedParent( node, Position.createAt( r1bQp, 0 ) );
 
 			expect( parent ).to.null;
 		} );
@@ -2866,7 +2866,7 @@ describe( 'SchemaContext', () => {
 		} );
 
 		it( 'creates context based on a position', () => {
-			const pos = Position.createAt( root.getChild( 0 ).getChild( 0 ) );
+			const pos = Position.createAt( root.getChild( 0 ).getChild( 0 ), 0 );
 			const ctx = new SchemaContext( pos );
 
 			expect( ctx.length ).to.equal( 3 );

+ 2 - 2
packages/ckeditor5-engine/tests/model/writer.js

@@ -1473,7 +1473,7 @@ describe( 'Writer', () => {
 			const docFrag = createDocumentFragment();
 
 			expect( () => {
-				move( range, docFrag );
+				move( range, docFrag, 0 );
 			} ).to.throw( CKEditorError, /^writer-move-different-document/ );
 		} );
 
@@ -2382,7 +2382,7 @@ describe( 'Writer', () => {
 			const firstParagraph = root.getNodeByPath( [ 1 ] );
 
 			const setFocusSpy = sinon.spy( DocumentSelection.prototype, '_setFocus' );
-			setSelectionFocus( firstParagraph );
+			setSelectionFocus( firstParagraph, 0 );
 
 			expect( setFocusSpy.calledOnce ).to.be.true;
 			setFocusSpy.restore();

+ 2 - 2
packages/ckeditor5-engine/tests/tickets/1281.js

@@ -47,10 +47,10 @@ describe( 'Bug ckeditor5-engine#1281', () => {
 
 		expect( selRanges.length ).to.equal( 2 );
 
-		assertPositions( Position.createAt( thirdParagraph ), selRanges[ 0 ].start );
+		assertPositions( Position.createAt( thirdParagraph, 0 ), selRanges[ 0 ].start );
 		assertPositions( Position.createAt( thirdParagraph, 'end' ), selRanges[ 0 ].end );
 
-		assertPositions( Position.createAt( fourthParagraph ), selRanges[ 1 ].start );
+		assertPositions( Position.createAt( fourthParagraph, 0 ), selRanges[ 1 ].start );
 		assertPositions( Position.createAt( fourthParagraph, 'end' ), selRanges[ 1 ].end );
 	} );
 

+ 2 - 2
packages/ckeditor5-engine/tests/view/position.js

@@ -167,8 +167,8 @@ describe( 'Position', () => {
 			const foo = new Text( 'foo' );
 			const p = new Element( 'p', null, foo );
 
-			expect( Position.createAt( foo ).parent ).to.equal( foo );
-			expect( Position.createAt( foo ).offset ).to.equal( 0 );
+			expect( Position.createAt( foo, 0 ).parent ).to.equal( foo );
+			expect( Position.createAt( foo, 0 ).offset ).to.equal( 0 );
 
 			expect( Position.createAt( foo, 2 ).parent ).to.equal( foo );
 			expect( Position.createAt( foo, 2 ).offset ).to.equal( 2 );