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

Merge branch 't/1060' into t/1015

Szymon Kupś 8 лет назад
Родитель
Сommit
1289d0673a

+ 42 - 0
packages/ckeditor5-engine/src/view/element.js

@@ -673,6 +673,37 @@ export default class Element extends Node {
 		yield* this._customProperties.entries();
 	}
 
+	/**
+	 * Returns identity string based on element's name, styles, classes and other attributes.
+	 * Two elements that {@link #isSimilar are similar} will have same identity string.
+	 * It has the following format:
+	 *
+	 *		"name|classes(class1,class2,class3)|styles(style1=val1,style2=val2)|attributes(attr1=val1,attr2=val2)"
+ 	 *
+	 * For example:
+	 *
+	 *		const element = new ViewElement( 'foo' );
+	 *		element.setAttribute( 'banana', '10' );
+	 *		element.setAttribute( 'apple', '20' );
+	 *		element.setStyle( 'color', 'red' );
+	 *		element.setStyle( 'border-color', 'white' );
+	 *		element.addClass( 'baz' );
+	 *
+	 *		// returns "foo|classes(baz)|styles(border-color=white,color=red)|attributes(apple=20,banana=10)"
+	 *		element.getIdentity();
+	 *
+	 * NOTE: Classes, styles and other attributes are sorted alphabetically.
+	 *
+	 * @returns {String}
+	 */
+	getIdentity() {
+		const classes = Array.from( this._classes ).sort().join( ',' );
+		const attributes = mapToSortedString( this._attrs );
+		const styles = mapToSortedString( this._styles );
+
+		return `${ this.name }|classes(${ classes })|styles(${ styles })|attributes(${ attributes })`;
+	}
+
 	/**
 	 * Returns block {@link module:engine/view/filler filler} offset or `null` if block filler is not needed.
 	 *
@@ -788,3 +819,14 @@ function normalize( nodes ) {
 			return typeof node == 'string' ? new Text( node ) : node;
 		} );
 }
+
+// Returns string representation of povided map in following format:
+//
+//		"mapKey1=value1,mapKey2=value2,mapKey3=value3"
+//
+// NOTE: All map keys should be strings. Key-value pairs will be sorted.
+//
+// @returns {String}
+function mapToSortedString( map ) {
+	return Array.from( map ).map( i => i[ 0 ] + '=' + i[ 1 ] ).sort().join( ',' );
+}

+ 26 - 1
packages/ckeditor5-engine/src/view/writer.js

@@ -900,7 +900,7 @@ function wrapChildren( parent, startOffset, endOffset, attribute ) {
 		const isUI = child.is( 'uiElement' );
 
 		// Wrap text, empty elements, ui elements or attributes with higher or equal priority.
-		if ( isText || isEmpty || isUI || ( isAttribute && attribute.priority <= child.priority ) ) {
+		if ( isText || isEmpty || isUI || ( isAttribute && shouldABeOutsideB( attribute, child ) ) ) {
 			// Clone attribute.
 			const newAttribute = attribute.clone();
 
@@ -942,6 +942,31 @@ function wrapChildren( parent, startOffset, endOffset, attribute ) {
 	return Range.createFromParentsAndOffsets( parent, startOffset, parent, endOffset );
 }
 
+// Checks if first {@link module:engine/view/attributeelement~AttributeElement AttributeElement} provided to the function
+// can be wrapped otuside second element. It is done by comparing elements'
+// {@link module:engine/view/attributeelement~AttributeElement#priority priorities}, if both have same priority
+// {@link module:engine/view/element~Element#getIdentity identities} are compared.
+//
+// @param {module:engine/view/attributeelement~AttributeElement} a
+// @param {module:engine/view/attributeelement~AttributeElement} b
+// @returns {Boolean}
+function shouldABeOutsideB( a, b ) {
+	if ( a.priority < b.priority ) {
+		return true;
+	} else if ( a.priority > b.priority ) {
+		return false;
+	}
+
+	// Two attribute elements with same priority and name will be merged into one element so there is no need to preserve
+	// order in such situations.
+	if ( a.name == b.name ) {
+		return true;
+	}
+
+	// When priorities are equal and names are different - use identities.
+	return a.getIdentity() < b.getIdentity();
+}
+
 // Returns new position that is moved to near text node. Returns same position if there is no text node before of after
 // specified position.
 //

+ 47 - 0
packages/ckeditor5-engine/tests/view/element.js

@@ -981,4 +981,51 @@ describe( 'Element', () => {
 			expect( properties[ 2 ][ 1 ] ).to.equal( 3 );
 		} );
 	} );
+
+	describe( 'getIdentity()', () => {
+		it( 'should return only name if no other attributes are present', () => {
+			const el = new Element( 'foo' );
+
+			expect( el.getIdentity() ).to.equal( 'foo|classes()|styles()|attributes()' );
+		} );
+
+		it( 'should return classes in sorted order', () => {
+			const el = new Element( 'fruit' );
+			el.addClass( 'banana', 'lemon', 'apple' );
+
+			expect( el.getIdentity() ).to.equal( 'fruit|classes(apple,banana,lemon)|styles()|attributes()' );
+		} );
+
+		it( 'should return styles in sorted order', () => {
+			const el = new Element( 'foo', {
+				style: 'border: 1px solid red; background-color: red'
+			} );
+
+			expect( el.getIdentity() ).to.equal( 'foo|classes()|styles(background-color=red,border=1px solid red)|attributes()' );
+		} );
+
+		it( 'should return attributes in sorted order', () => {
+			const el = new Element( 'foo', {
+				a: 1,
+				d: 4,
+				b: 3
+			} );
+
+			expect( el.getIdentity() ).to.equal( 'foo|classes()|styles()|attributes(a=1,b=3,d=4)' );
+		} );
+
+		it( 'should return classes, styles and attributes', () => {
+			const el = new Element( 'baz', {
+				foo: 'one',
+				bar: 'two',
+				style: 'text-align:center;border-radius:10px'
+			} );
+
+			el.addClass( 'three', 'two', 'one' );
+
+			expect( el.getIdentity() ).to.equal(
+				'baz|classes(one,three,two)|styles(border-radius=10px,text-align=center)|attributes(bar=two,foo=one)'
+			);
+		} );
+	} );
 } );

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

@@ -318,5 +318,27 @@ describe( 'writer', () => {
 				wrap( range, new AttributeElement( 'b' ) );
 			} ).to.throw( CKEditorError, 'view-writer-cannot-break-ui-element' );
 		} );
+
+		it( 'should keep stable hierarchy when wrapping with attribute with same priority', () => {
+			test(
+				'<container:p>[<attribute:span>foo</attribute:span>]</container:p>',
+				'<attribute:b></attribute:b>',
+				'<container:p>' +
+					'[<attribute:b view-priority="10">' +
+						'<attribute:span view-priority="10">foo</attribute:span>' +
+					'</attribute:b>]' +
+				'</container:p>'
+			);
+
+			test(
+				'<container:p>[<attribute:b>foo</attribute:b>]</container:p>',
+				'<attribute:span></attribute:span>',
+				'<container:p>' +
+					'[<attribute:b view-priority="10">' +
+						'<attribute:span view-priority="10">foo</attribute:span>' +
+					'</attribute:b>]' +
+				'</container:p>'
+			);
+		} );
 	} );
 } );

+ 5 - 3
packages/ckeditor5-engine/tests/view/writer/wrapposition.js

@@ -101,9 +101,11 @@ describe( 'wrapPosition', () => {
 			'<container:p><attribute:b view-priority="1">foo{}bar</attribute:b></container:p>',
 			'<attribute:u view-priority="1"></attribute:u>',
 			'<container:p>' +
-				'<attribute:b view-priority="1">foo</attribute:b>' +
-				'<attribute:u view-priority="1"><attribute:b view-priority="1">[]</attribute:b></attribute:u>' +
-				'<attribute:b view-priority="1">bar</attribute:b>' +
+				'<attribute:b view-priority="1">' +
+					'foo' +
+					'<attribute:u view-priority="1">[]</attribute:u>' +
+					'bar' +
+				'</attribute:b>' +
 			'</container:p>'
 		);
 	} );