Browse Source

Added tests and docs to treeView.Writer.wrap method.

Szymon Kupś 9 years ago
parent
commit
39feaa2a41

+ 26 - 11
packages/ckeditor5-engine/src/treeview/writer.js

@@ -309,7 +309,7 @@ import DocumentFragment from './documentfragment.js';
 	}
 	}
 
 
 	/**
 	/**
-	 * Wraps elements within range with provided attribute element.
+	 * Wraps elements within range with provided {@link engine.treeView.AttributeElement AttributeElement}.
 	 *
 	 *
 	 * Throws {@link utils.CKEditorError} `treeview-writer-invalid-range-container` when {@link engine.treeView.Range#start}
 	 * Throws {@link utils.CKEditorError} `treeview-writer-invalid-range-container` when {@link engine.treeView.Range#start}
 	 * and {@link engine.treeView.Range#end} positions are not placed inside same parent container.
 	 * and {@link engine.treeView.Range#end} positions are not placed inside same parent container.
@@ -354,15 +354,11 @@ import DocumentFragment from './documentfragment.js';
 		}
 		}
 
 
 		// Range is inside single attribute and spans on all children.
 		// Range is inside single attribute and spans on all children.
-		if ( range.start.parent == range.end.parent && range.end.parent instanceof AttributeElement ) {
-			if ( range.start.offset === 0 && range.end.offset === range.start.parent.getChildCount() ) {
-				if ( wrapAttributes( attribute, range.start.parent ) ) {
-					const parent = range.start.parent.parent;
-					const index = range.start.parent.getIndex();
-
-					return Range.createFromParentsAndOffsets( parent, index, parent, index + 1 ) ;
-				}
-			}
+		if ( rangeSpansOnAllChildren( range ) && wrapAttributes( attribute, range.start.parent ) ) {
+			const parent = range.start.parent.parent;
+			const index = range.start.parent.getIndex();
+
+			return Range.createFromParentsAndOffsets( parent, index, parent, index + 1 ) ;
 		}
 		}
 
 
 		// Break attributes at range start and end.
 		// Break attributes at range start and end.
@@ -702,6 +698,14 @@ function mergeTextNodes( t1, t2 ) {
 	return new Position( t1, nodeBeforeLength );
 	return new Position( t1, nodeBeforeLength );
 }
 }
 
 
+// Wraps one {@link engine.treeView.AttributeElement AttributeElement} into another by merging them if possible.
+// Two AttributeElements can be merged when there is no attribute or style conflicts between them.
+// When merging is possible - all attributes, styles and classes are moved from wrapper element to element being
+// wrapped.
+//
+// @param {engine.treeView.AttributeElement} wrapper Wrapper AttributeElement.
+// @param {engine.treeView.AttributeElement} toWrap AttributeElement to wrap using wrapper element.
+// @returns {Boolean} Returns `true` if elements are merged.
 function wrapAttributes( wrapper, toWrap ) {
 function wrapAttributes( wrapper, toWrap ) {
 	// Can't merge if name or priority differs.
 	// Can't merge if name or priority differs.
 	if ( wrapper.name !== toWrap.name || wrapper.priority !== toWrap.priority ) {
 	if ( wrapper.name !== toWrap.name || wrapper.priority !== toWrap.priority ) {
@@ -728,7 +732,7 @@ function wrapAttributes( wrapper, toWrap ) {
 		}
 		}
 	}
 	}
 
 
-	// Move all attributes/classes/styles from wrapper to wrapped attribute.
+	// Move all attributes/classes/styles from wrapper to wrapped AttributeElement.
 	for ( let key of wrapper.getAttributeKeys() ) {
 	for ( let key of wrapper.getAttributeKeys() ) {
 		// Classes and styles should be checked separately.
 		// Classes and styles should be checked separately.
 		if ( key === 'class' || key === 'style' ) {
 		if ( key === 'class' || key === 'style' ) {
@@ -754,4 +758,15 @@ function wrapAttributes( wrapper, toWrap ) {
 	}
 	}
 
 
 	return true;
 	return true;
+}
+
+// Returns `true` if range is located in same {@link engine.treeView.AttributeElement AttributeElement}
+// (`start` and `end` positions are located inside same {@link engine.treeView.AttributeElement AttributeElement}),
+// starts on 0 offset and ends after last child node.
+//
+// @param {engine.treeView.Range} Range
+// @returns {Boolean}
+function rangeSpansOnAllChildren( range ) {
+	return range.start.parent == range.end.parent && range.start.parent instanceof AttributeElement &&
+		range.start.offset === 0 && range.end.offset === range.start.parent.getChildCount();
 }
 }

+ 97 - 3
packages/ckeditor5-engine/tests/treeview/writer/wrap.js

@@ -552,7 +552,7 @@ describe( 'Writer', () => {
 			} );
 			} );
 		} );
 		} );
 
 
-		it( 'should wrap single element by joining attributes', () => {
+		it( 'should wrap single element by merging attributes', () => {
 			// <p>[<b foo="bar" one="two"></b>]</p>
 			// <p>[<b foo="bar" one="two"></b>]</p>
 			// wrap with <b baz="qux" one="two"></b>
 			// wrap with <b baz="qux" one="two"></b>
 			// <p>[<b foo="bar" one="two" baz="qux"></b>]</p>
 			// <p>[<b foo="bar" one="two" baz="qux"></b>]</p>
@@ -583,7 +583,38 @@ describe( 'Writer', () => {
 			} );
 			} );
 		} );
 		} );
 
 
-		it( 'should wrap single element by joining classes', () => {
+		it( 'should not merge attributes when they differ', () => {
+			// <p>[<b foo="bar" ></b>]</p>
+			// wrap with <b foo="baz"></b>
+			// <p>[<b foo="baz"><b foo="bar"></b></b>]</p>
+			const b = new AttributeElement( 'b', {
+				foo: 'bar'
+			} );
+			const p = new ContainerElement( 'p', null, b );
+			const range = Range.createFromParentsAndOffsets( p, 0, p, 1 );
+			const wrapper = new AttributeElement( 'b', {
+				foo: 'baz'
+			} );
+
+			const newRange = writer.wrap( range, wrapper );
+			expect( b.getAttribute( 'foo' ) ).to.equal( 'bar' );
+			expect( b.parent.isSimilar( wrapper ) ).to.be.true;
+			expect( b.parent.getAttribute( 'foo' ) ).to.equal( 'baz' );
+
+			test( writer, newRange, p, {
+				instanceOf: ContainerElement,
+				name: 'p',
+				rangeStart: 0,
+				rangeEnd: 1,
+				children: [
+					{ instanceOf: AttributeElement, name: 'b', children: [
+						{ instanceOf: AttributeElement, name: 'b', children: [] }
+					] }
+				]
+			} );
+		} );
+
+		it( 'should wrap single element by merging classes', () => {
 			// <p>[<b class="foo bar baz" ></b>]</p>
 			// <p>[<b class="foo bar baz" ></b>]</p>
 			// wrap with <b class="foo bar qux jax"></b>
 			// wrap with <b class="foo bar qux jax"></b>
 			// <p>[<b class="foo bar baz qux jax"></b>]</p>
 			// <p>[<b class="foo bar baz qux jax"></b>]</p>
@@ -609,7 +640,7 @@ describe( 'Writer', () => {
 			} );
 			} );
 		} );
 		} );
 
 
-		it( 'should wrap single element by joining styles', () => {
+		it( 'should wrap single element by merging styles', () => {
 			// <p>[<b style="color:red; position: absolute;"></b>]</p>
 			// <p>[<b style="color:red; position: absolute;"></b>]</p>
 			// wrap with <b style="color:red; top: 20px;"></b>
 			// wrap with <b style="color:red; top: 20px;"></b>
 			// <p>[<b class="color:red; position: absolute; top:20px;"></b>]</p>
 			// <p>[<b class="color:red; position: absolute; top:20px;"></b>]</p>
@@ -638,6 +669,69 @@ describe( 'Writer', () => {
 			} );
 			} );
 		} );
 		} );
 
 
+		it( 'should not merge styles when they differ', () => {
+			// <p>[<b style="color:red;"></b>]</p>
+			// wrap with <b style="color:black;"></b>
+			// <p>[<b style="color:black;"><b style="color:red;"></b></b>]</p>
+			const b = new AttributeElement( 'b', {
+				style: 'color:red'
+			} );
+			const p = new ContainerElement( 'p', null, b );
+			const range = Range.createFromParentsAndOffsets( p, 0, p, 1 );
+			const wrapper = new AttributeElement( 'b', {
+				style: 'color:black'
+			} );
+
+			const newRange = writer.wrap( range, wrapper );
+			expect( b.getStyle( 'color' ) ).to.equal( 'red' );
+			expect( b.parent.isSimilar( wrapper ) ).to.be.true;
+			expect( b.parent.getStyle( 'color' ) ).to.equal( 'black' );
+
+			test( writer, newRange, p, {
+				instanceOf: ContainerElement,
+				name: 'p',
+				rangeStart: 0,
+				rangeEnd: 1,
+				children: [
+					{ instanceOf: AttributeElement, name: 'b', children: [
+						{ instanceOf: AttributeElement, name: 'b', children: [] }
+					] }
+				]
+			} );
+		} );
+
+		it( 'should not merge single elements when they have different priority', () => {
+			// <p>[<b style="color:red;"></b>]</p>
+			// wrap with <b style="color:red;"></b> with different priority
+			// <p>[<b style="color:red;"><b style="color:red;"></b></b>]</p>
+			const b = new AttributeElement( 'b', {
+				style: 'color:red'
+			} );
+			const p = new ContainerElement( 'p', null, b );
+			const range = Range.createFromParentsAndOffsets( p, 0, p, 1 );
+			const wrapper = new AttributeElement( 'b', {
+				style: 'color:red'
+			} );
+			wrapper.priority = b.priority - 1;
+
+			const newRange = writer.wrap( range, wrapper );
+			expect( b.getStyle( 'color' ) ).to.equal( 'red' );
+			expect( b.parent.isSimilar( wrapper ) ).to.be.true;
+			expect( b.parent.getStyle( 'color' ) ).to.equal( 'red' );
+
+			test( writer, newRange, p, {
+				instanceOf: ContainerElement,
+				name: 'p',
+				rangeStart: 0,
+				rangeEnd: 1,
+				children: [
+					{ instanceOf: AttributeElement, name: 'b', children: [
+						{ instanceOf: AttributeElement, name: 'b', children: [] }
+					] }
+				]
+			} );
+		} );
+
 		it( 'should be merged with outside element when wrapping all children', () => {
 		it( 'should be merged with outside element when wrapping all children', () => {
 			// <p><b foo="bar">[{foobar}<i>{baz}</i>]</b></p>
 			// <p><b foo="bar">[{foobar}<i>{baz}</i>]</b></p>
 			// wrap with <b baz="qux"></b>
 			// wrap with <b baz="qux"></b>