Преглед изворни кода

Merge branch 'master' into t/396

Maciej Gołaszewski пре 9 година
родитељ
комит
1be3614b91

+ 3 - 4
packages/ckeditor5-engine/src/treecontroller/model-selection-to-view-converters.js

@@ -198,13 +198,12 @@ export function convertSelectionAttribute( elementCreator ) {
 export function clearAttributes() {
 	return ( evt, data, consumable, conversionApi ) => {
 		for ( let range of conversionApi.viewSelection.getRanges() ) {
-			const parentNode = range.start.parent;
+			conversionApi.writer.mergeAttributes( range.end );
 
-			if ( parentNode instanceof ViewElement && parentNode.getChildCount() === 0 ) {
-				parentNode.parent.removeChildren( parentNode.getIndex() );
+			if ( !range.isCollapsed ) {
+				conversionApi.writer.mergeAttributes( range.start );
 			}
 		}
-
 		conversionApi.viewSelection.removeAllRanges();
 	};
 }

+ 14 - 0
packages/ckeditor5-engine/src/treeview/writer.js

@@ -204,6 +204,11 @@ import isIterable from '../../utils/isiterable.js';
 	 *		<p><b>foo</b>[]<b>bar</b> -> <p><b>foo{}bar</b></b>
 	 *		<p><b foo="bar">a</b>[]<b foo="baz">b</b> -> <p><b foo="bar">a</b>[]<b foo="baz">b</b>
 	 *
+	 * It will also take care about empty attributes when merging:
+	 *
+	 *		<p><b>[]</b></p> -> <p>[]</p>
+	 *		<p><b>foo</b><i>[]</i><b>bar</b></p> -> <p><b>foo{}bar</b></p>
+	 *
 	 * @see engine.treeView.AttributeElement
 	 * @see engine.treeView.ContainerElement
 	 * @param {engine.treeView.Position} position Merge position.
@@ -218,6 +223,15 @@ import isIterable from '../../utils/isiterable.js';
 			return position;
 		}
 
+		// When inside empty attribute - remove it.
+		if ( positionParent instanceof AttributeElement && positionParent.getChildCount() === 0 ) {
+			const parent = positionParent.parent;
+			const offset = positionParent.getIndex();
+			positionParent.remove();
+
+			return this.mergeAttributes( new Position( parent, offset ) );
+		}
+
 		const nodeBefore = positionParent.getChild( positionOffset - 1 );
 		const nodeAfter = positionParent.getChild( positionOffset );
 

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

@@ -118,7 +118,7 @@ describe( 'view test utils', () => {
 			expect( stringify( p ) ).to.equal( '<p><b>foobar</b></p>' );
 		} );
 
-		it( 'should write elements with attributes', () => {
+		it( 'should write elements with attributes (attributes in alphabetical order)', () => {
 			const text = new Text( 'foobar' );
 			const b = new Element( 'b', {
 				foo: 'bar'
@@ -129,7 +129,7 @@ describe( 'view test utils', () => {
 				class: 'short wide'
 			}, b );
 
-			expect( stringify( p ) ).to.equal( '<p class="short wide" baz="qux" bar="taz"><b foo="bar">foobar</b></p>' );
+			expect( stringify( p ) ).to.equal( '<p bar="taz" baz="qux" class="short wide"><b foo="bar">foobar</b></p>' );
 		} );
 
 		it( 'should write selection ranges inside elements', () => {

+ 2 - 1
packages/ckeditor5-engine/tests/_utils/view.js

@@ -913,8 +913,9 @@ class ViewStringify {
 	 */
 	_stringifyElementAttributes( element ) {
 		const attributes = [];
+		const keys = [ ...element.getAttributeKeys() ].sort();
 
-		for ( let attribute of element.getAttributeKeys() ) {
+		for ( let attribute of keys ) {
 			attributes.push( `${ attribute }="${ element.getAttribute( attribute ) }"` );
 		}
 

+ 23 - 2
packages/ckeditor5-engine/tests/treeview/writer/mergeattributes.js

@@ -79,8 +79,8 @@ describe( 'Writer', () => {
 
 		it( 'should merge when placed between similar attribute nodes', () => {
 			test(
-				'<container:p><attribute:b:1 foo="bar"></attribute:b:1>[]<attribute:b:1 foo="bar"></attribute:b:1></container:p>',
-				'<container:p><attribute:b:1 foo="bar">[]</attribute:b:1></container:p>'
+				'<container:p><attribute:b:1 foo="bar">baz</attribute:b:1>[]<attribute:b:1 foo="bar">qux</attribute:b:1></container:p>',
+				'<container:p><attribute:b:1 foo="bar">baz{}qux</attribute:b:1></container:p>'
 			);
 		} );
 
@@ -104,5 +104,26 @@ describe( 'Writer', () => {
 				'<container:p><attribute:b:1 foo="bar">foo{}bar</attribute:b:1></container:p>'
 			);
 		} );
+
+		it( 'should remove empty attributes after merge #1', () => {
+			test(
+				'<container:p><attribute:b>[]</attribute:b></container:p>',
+				'<container:p>[]</container:p>'
+			);
+		} );
+
+		it( 'should remove empty attributes after merge #2', () => {
+			test(
+				'<container:p><attribute:b>foo</attribute:b><attribute:i>[]</attribute:i><attribute:b>bar</attribute:b></container:p>',
+				'<container:p><attribute:b:10>foo{}bar</attribute:b:10></container:p>'
+			);
+		} );
+
+		it( 'should remove empty attributes after merge #3', () => {
+			test(
+				'<container:p><attribute:b></attribute:b><attribute:i>[]</attribute:i><attribute:b></attribute:b></container:p>',
+				'<container:p>[]</container:p>'
+			);
+		} );
 	} );
 } );

+ 2 - 2
packages/ckeditor5-engine/tests/treeview/writer/unwrap.js

@@ -237,9 +237,9 @@ describe( 'Writer', () => {
 
 		it( 'should not unwrap single element when attributes are different', () => {
 			test(
-				'<container:p>[<attribute:b:1 foo="bar" baz="qux">test</attribute:b:1>]</container:p>',
+				'<container:p>[<attribute:b:1 baz="qux" foo="bar">test</attribute:b:1>]</container:p>',
 				'<attribute:b:1 baz="qux" test="true"></attribute:b:1>',
-				'<container:p>[<attribute:b:1 foo="bar" baz="qux">test</attribute:b:1>]</container:p>'
+				'<container:p>[<attribute:b:1 baz="qux" foo="bar">test</attribute:b:1>]</container:p>'
 			);
 		} );
 

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

@@ -173,7 +173,7 @@ describe( 'Writer', () => {
 			test(
 				'<container:p>[<attribute:b:1 foo="bar" one="two"></attribute:b:1>]</container:p>',
 				'<attribute:b:1 baz="qux" one="two"></attribute:b:1>',
-				'<container:p>[<attribute:b:1 foo="bar" one="two" baz="qux"></attribute:b:1>]</container:p>'
+				'<container:p>[<attribute:b:1 baz="qux" foo="bar" one="two"></attribute:b:1>]</container:p>'
 			);
 		} );
 
@@ -234,7 +234,7 @@ describe( 'Writer', () => {
 				'<attribute:b:1 baz="qux"></attribute:b:1>',
 				'<container:p>' +
 				'[' +
-					'<attribute:b:1 foo="bar" baz="qux">' +
+					'<attribute:b:1 baz="qux" foo="bar">' +
 						'foobar' +
 						'<attribute:i:1>baz</attribute:i:1>' +
 					'</attribute:b:1>' +