8
0
Просмотр исходного кода

View writer now throws when want to break attributes inside EmptyElement. Added tests.

Szymon Kupś 9 лет назад
Родитель
Сommit
611f303796

+ 17 - 5
packages/ckeditor5-engine/src/view/writer.js

@@ -55,6 +55,9 @@ export default {
  * Throws {@link utils.CKEditorError CKEditorError} `view-writer-invalid-range-container` when {@link engine.view.Range#start start}
  * and {@link engine.view.Range#end end} positions of a passed range are not placed inside same parent container.
  *
+ * Throws {@link utils.CKEditorError CKEditorError} `view-writer-cannot-break-empty-element` when trying to break attributes
+ * inside {@link engine.view.EmptyElement EmptyElement}.
+ *
  * @see engine.view.AttributeElement
  * @see engine.view.ContainerElement
  * @see engine.view.writer.breakContainer
@@ -636,6 +639,9 @@ function _breakAttributesRange( range, forceSplitText = false ) {
 // Function used by public breakAttributes (without splitting text nodes) and by other methods (with
 // splitting text nodes).
 //
+// Throws {@link utils.CKEditorError CKEditorError} `view-writer-cannot-break-empty-element` break position is placed
+// inside {@link engine.view.EmptyElement EmptyElement}.
+//
 // @param {engine.view.Position} position Position where to break attributes.
 // @param {Boolean} [forceSplitText = false] If set to `true`, will break text nodes even if they are directly in
 // container element. This behavior will result in incorrect view state, but is needed by other view writing methods
@@ -645,9 +651,14 @@ function _breakAttributes( position, forceSplitText = false ) {
 	const positionOffset = position.offset;
 	const positionParent = position.parent;
 
-	// If position is placed inside EmptyElement - return same position, cannot break EmptyElement.
-	if ( positionParent instanceof EmptyElement ) {
-		return Position.createFromPosition( position );
+	// If position is placed inside EmptyElement - throw an exception as we cannot break inside.
+	if ( position.parent instanceof EmptyElement ) {
+		/**
+		 * Cannot break inside EmptyElement instance.
+		 *
+		 * @error view-writer-cannot-break-empty-element
+		 */
+		throw new CKEditorError( 'view-writer-cannot-break-empty-element' );
 	}
 
 	// There are no attributes to break and text nodes breaking is not forced.
@@ -792,9 +803,10 @@ function wrapChildren( parent, startOffset, endOffset, attribute ) {
 		const child = parent.getChild( i );
 		const isText = child instanceof Text;
 		const isAttribute = child instanceof AttributeElement;
+		const isEmpty = child instanceof EmptyElement;
 
-		// Wrap text or attributes with higher or equal priority.
-		if ( isText || ( isAttribute && attribute.priority <= child.priority ) ) {
+		// Wrap text, empty elements or attributes with higher or equal priority.
+		if ( isText || isEmpty || ( isAttribute && attribute.priority <= child.priority ) ) {
 			// Clone attribute.
 			const newAttribute = attribute.clone();
 

+ 16 - 7
packages/ckeditor5-engine/tests/view/writer/breakAttributes.js

@@ -230,16 +230,25 @@ describe( 'writer', () => {
 				);
 			} );
 
-			it( 'should not break EmptyElements', () => {
+			it( 'should throw if breaking inside EmptyElement #1', () => {
 				const img = new EmptyElement( 'img' );
-				const container = new ContainerElement( 'p', null, [ img ] );
+				new ContainerElement( 'p', null, img );
 				const position = new Position( img, 0 );
 
-				const newPosition = breakAttributes( position );
-				expect( stringify( container, newPosition, {
-					showType: true,
-					showPriority: true
-				} ) ).to.equal( '<container:p><empty:img>[]</empty:img></container:p>' );
+				expect( () => {
+					breakAttributes( position );
+				} ).to.throw( CKEditorError, 'view-writer-cannot-break-empty-element' );
+			} );
+
+			it( 'should throw if breaking inside EmptyElement #2', () => {
+				const img = new EmptyElement( 'img' );
+				const b = new AttributeElement( 'b' );
+				new ContainerElement( 'p', null, [ img, b ] );
+				const range = Range.createFromParentsAndOffsets( img, 0, b, 0 );
+
+				expect( () => {
+					breakAttributes( range );
+				} ).to.throw( CKEditorError, 'view-writer-cannot-break-empty-element' );
 			} );
 		} );
 	} );

+ 12 - 0
packages/ckeditor5-engine/tests/view/writer/insert.js

@@ -8,6 +8,7 @@
 import { insert } from 'ckeditor5/engine/view/writer.js';
 import ContainerElement from 'ckeditor5/engine/view/containerelement.js';
 import Element from 'ckeditor5/engine/view/element.js';
+import EmptyElement from 'ckeditor5/engine/view/emptyelement.js';
 import Position from 'ckeditor5/engine/view/position.js';
 import CKEditorError from 'ckeditor5/utils/ckeditorerror.js';
 import { stringify, parse } from 'ckeditor5/engine/dev-utils/view.js';
@@ -183,5 +184,16 @@ describe( 'writer', () => {
 				'<container:p>[<empty:img></empty:img>]</container:p>'
 			);
 		} );
+
+		it( 'should throw if trying to insert inside EmptyElement', () => {
+			const emptyElement = new EmptyElement( 'img' );
+			new ContainerElement( 'p', null, emptyElement );
+			const position = new Position( emptyElement, 0 );
+			const attributeElement = new AttributeElement( 'i' );
+
+			expect( () => {
+				insert( position, attributeElement );
+			} ).to.throw( CKEditorError, 'view-writer-cannot-break-empty-element' );
+		} );
 	} );
 } );

+ 29 - 0
packages/ckeditor5-engine/tests/view/writer/move.js

@@ -8,6 +8,12 @@
 import { move } from 'ckeditor5/engine/view/writer.js';
 import ViewPosition from 'ckeditor5/engine/view/position.js';
 import { stringify, parse } from 'ckeditor5/engine/dev-utils/view.js';
+import ContainerElement from 'ckeditor5/engine/view/containerelement.js';
+import AttributeElement from 'ckeditor5/engine/view/attributeelement.js';
+import EmptyElement from 'ckeditor5/engine/view/emptyelement.js';
+import Range from 'ckeditor5/engine/view/range.js';
+import Position from 'ckeditor5/engine/view/position.js';
+import CKEditorError from 'ckeditor5/utils/ckeditorerror.js';
 
 describe( 'writer', () => {
 	/**
@@ -112,5 +118,28 @@ describe( 'writer', () => {
 			const expectedView = '<container:p>b[<attribute:b>a}c</attribute:b></container:p>';
 			expect( stringify( view, newRange, { showType: true } ) ).to.equal( expectedView );
 		} );
+
+		it( 'should move EmptyElement', () => {
+			test(
+				'<container:p>foo[<empty:img></empty:img>]bar</container:p>',
+				'<container:div>baz{}quix</container:div>',
+				'<container:p>foobar</container:p>',
+				'<container:div>baz[<empty:img></empty:img>]quix</container:div>'
+			);
+		} );
+
+		it( 'should throw if trying to move to EmptyElement', () => {
+			const srcAttribute = new AttributeElement( 'b' );
+			const srcContainer = new ContainerElement( 'p', null, srcAttribute );
+			const srcRange = Range.createFromParentsAndOffsets( srcContainer, 0, srcContainer, 1 );
+
+			const dstEmpty = new EmptyElement( 'img' );
+			new ContainerElement( 'p', null, dstEmpty );
+			const dstPosition = new Position( dstEmpty, 0 );
+
+			expect( () => {
+				move( srcRange, dstPosition );
+			} ).to.throw( CKEditorError, 'view-writer-cannot-break-empty-element' );
+		} );
 	} );
 } );

+ 20 - 0
packages/ckeditor5-engine/tests/view/writer/remove.js

@@ -11,6 +11,7 @@ import Range from 'ckeditor5/engine/view/range.js';
 import DocumentFragment from 'ckeditor5/engine/view/documentfragment.js';
 import { stringify, parse } from 'ckeditor5/engine/dev-utils/view.js';
 import AttributeElement from 'ckeditor5/engine/view/attributeelement.js';
+import EmptyElement from 'ckeditor5/engine/view/emptyelement.js';
 import CKEditorError from 'ckeditor5/utils/ckeditorerror.js';
 
 describe( 'writer', () => {
@@ -110,5 +111,24 @@ describe( 'writer', () => {
 		it( 'should remove part of the text node in document fragment', () => {
 			test( 'fo{ob}ar', 'fo{}ar', 'ob' );
 		} );
+
+		it( 'should remove EmptyElement', () => {
+			test(
+				'<container:p>foo[<empty:img></empty:img>]bar</container:p>',
+				'<container:p>foo{}bar</container:p>',
+				'<empty:img></empty:img>'
+			);
+		} );
+
+		it( 'should throw if range is placed inside EmptyElement', () => {
+			const emptyElement = new EmptyElement( 'img' );
+			const attributeElement = new AttributeElement( 'b' );
+			new ContainerElement( 'p', null, [ emptyElement, attributeElement ] );
+			const range = Range.createFromParentsAndOffsets( emptyElement, 0, attributeElement, 0 );
+
+			expect( () => {
+				remove( range );
+			} ).to.throw( CKEditorError, 'view-writer-cannot-break-empty-element' );
+		} );
 	} );
 } );

+ 20 - 0
packages/ckeditor5-engine/tests/view/writer/unwrap.js

@@ -9,6 +9,7 @@ import { unwrap } from 'ckeditor5/engine/view/writer.js';
 import Element from 'ckeditor5/engine/view/element.js';
 import ContainerElement from 'ckeditor5/engine/view/containerelement.js';
 import AttributeElement from 'ckeditor5/engine/view/attributeelement.js';
+import EmptyElement from 'ckeditor5/engine/view/emptyelement.js';
 import Position from 'ckeditor5/engine/view/position.js';
 import Range from 'ckeditor5/engine/view/range.js';
 import Text from 'ckeditor5/engine/view/text.js';
@@ -298,5 +299,24 @@ describe( 'writer', () => {
 				'<container:p>[foobar]</container:p>'
 			);
 		} );
+
+		it( 'should unwrap EmptyElement', () => {
+			test(
+				'<container:p>[<attribute:b><empty:img></empty:img></attribute:b>]</container:p>',
+				'<attribute:b></attribute:b>',
+				'<container:p>[<empty:img></empty:img>]</container:p>'
+			);
+		} );
+
+		it( 'should throw if range is inside EmptyElement', () => {
+			const empty = new EmptyElement( 'img' );
+			const attribute = new AttributeElement( 'b' );
+			const container = new ContainerElement( 'p', null, [ empty, attribute ] );
+			const range = Range.createFromParentsAndOffsets( empty, 0, container, 2 );
+
+			expect( () => {
+				unwrap( range, attribute );
+			} ).to.throw( CKEditorError, 'view-writer-cannot-break-empty-element' );
+		} );
 	} );
 } );

+ 19 - 0
packages/ckeditor5-engine/tests/view/writer/wrap.js

@@ -9,6 +9,7 @@ import { wrap } from 'ckeditor5/engine/view/writer.js';
 import Element from 'ckeditor5/engine/view/element.js';
 import ContainerElement from 'ckeditor5/engine/view/containerelement.js';
 import AttributeElement from 'ckeditor5/engine/view/attributeelement.js';
+import EmptyElement from 'ckeditor5/engine/view/emptyelement.js';
 import Position from 'ckeditor5/engine/view/position.js';
 import Range from 'ckeditor5/engine/view/range.js';
 import Text from 'ckeditor5/engine/view/text.js';
@@ -270,5 +271,23 @@ describe( 'writer', () => {
 				'</container:p>'
 			);
 		} );
+
+		it( 'should wrap EmptyElement', () => {
+			test(
+				'<container:p>[<empty:img></empty:img>]</container:p>',
+				'<attribute:b></attribute:b>',
+				'<container:p>[<attribute:b view-priority="10"><empty:img></empty:img></attribute:b>]</container:p>'
+			);
+		} );
+
+		it( 'should throw if range is inside EmptyElement', () => {
+			const emptyElement = new EmptyElement( 'img' );
+			const container = new ContainerElement( 'p', null, emptyElement );
+			const range = Range.createFromParentsAndOffsets( emptyElement, 0, container, 1 );
+
+			expect( () => {
+				wrap( range, new AttributeElement( 'b' ) );
+			} ).to.throw( CKEditorError, 'view-writer-cannot-break-empty-element' );
+		} );
 	} );
 } );

+ 13 - 0
packages/ckeditor5-engine/tests/view/writer/wrapposition.js

@@ -9,6 +9,8 @@ import { wrapPosition } from 'ckeditor5/engine/view/writer.js';
 import Text from 'ckeditor5/engine/view/text.js';
 import Element from 'ckeditor5/engine/view/element.js';
 import ContainerElement from 'ckeditor5/engine/view/containerelement.js';
+import AttributeElement from 'ckeditor5/engine/view/attributeelement.js';
+import EmptyElement from 'ckeditor5/engine/view/emptyelement.js';
 import Position from 'ckeditor5/engine/view/position.js';
 import CKEditorError from 'ckeditor5/utils/ckeditorerror.js';
 import { stringify, parse } from 'ckeditor5/engine/dev-utils/view.js';
@@ -122,4 +124,15 @@ describe( 'wrapPosition', () => {
 			'<container:p><attribute:b view-priority="1">foobar{}</attribute:b></container:p>'
 		);
 	} );
+
+	it( 'should throw if position is set inside EmptyElement', () => {
+		const emptyElement = new EmptyElement( 'img' );
+		new ContainerElement( 'p', null, emptyElement );
+		const attributeElement = new AttributeElement( 'b' );
+		const position = new Position( emptyElement, 0 );
+
+		expect( () => {
+			wrapPosition( position, attributeElement );
+		} ).to.throw( CKEditorError, 'view-emptyelement-cannot-add' );
+	} );
 } );