DomConverter should not reverse order of attributes while converting …

…from DOM to view.
ckeditor · Jun 21, 2022 · 2a22cd9 · 2a22cd9
1 parent f921c06
commit 2a22cd9
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 9 deletions.
diff --git a/packages/ckeditor5-engine/src/view/domconverter.js b/packages/ckeditor5-engine/src/view/domconverter.js
@@ -680,7 +680,7 @@ export default class DomConverter {
 				const attrs = domNode.attributes;
 
 				if ( attrs ) {
-					for ( let i = attrs.length - 1; i >= 0; i-- ) {
+					for ( let l = attrs.length, i = 0; i < l; i++ ) {
 						viewElement._setAttribute( attrs[ i ].name, attrs[ i ].value );
 					}
 				}

diff --git a/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js b/packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js
@@ -206,6 +206,22 @@ describe( 'DomConverter', () => {
 			expect( viewComment ).to.be.null;
 		} );
 
+		it( 'should set attributes in the same order as in the DOM', () => {
+			const domP = createElement( document, 'p', { 'data-foo': 'a', 'data-bar': 'b' } );
+			const viewP = converter.domToView( domP );
+
+			expect( viewP ).to.be.an.instanceof( ViewElement );
+			expect( viewP.name ).to.equal( 'p' );
+
+			const attributes = Array.from( viewP.getAttributes() );
+
+			expect( attributes.length ).to.equal( 2 );
+			expect( attributes ).to.deep.equal( [
+				[ 'data-foo', 'a' ],
+				[ 'data-bar', 'b' ]
+			] );
+		} );
+
 		describe( 'it should clear whitespaces', () => {
 			it( 'at the beginning of block element', () => {
 				const domDiv = createElement( document, 'div', {}, [

diff --git a/packages/ckeditor5-html-support/tests/datafilter.js b/packages/ckeditor5-html-support/tests/datafilter.js
@@ -605,6 +605,24 @@ describe( 'DataFilter', () => {
 			);
 		} );
 
+		it( 'should not change order of attributes', () => {
+			dataFilter.allowElement( 'section' );
+			dataFilter.allowAttributes( {
+				name: 'section',
+				attributes: true
+			} );
+
+			editor.setData( '<section data-foo="a" data-bar="b"><p>foobar</p></section>' );
+
+			expect( getModelData( model, { withoutSelection: true } ) ).to.deep.equal(
+				'<htmlSection htmlAttributes="{"attributes":{"data-foo":"a","data-bar":"b"}}"><paragraph>foobar</paragraph></htmlSection>'
+			);
+
+			expect( editor.getData() ).to.equal(
+				'<section data-foo="a" data-bar="b"><p>foobar</p></section>'
+			);
+		} );
+
 		it( 'should disallow attributes', () => {
 			dataFilter.allowElement( 'section' );
 			dataFilter.allowAttributes( { name: 'section', attributes: { 'data-foo': /[\s\S]+/ } } );
@@ -1765,7 +1783,7 @@ describe( 'DataFilter', () => {
 				} );
 
 				expect( editor.getData() ).to.equal(
-					'<p><input data-bar="bar" data-foo="baz"></p>'
+					'<p><input data-foo="baz" data-bar="bar"></p>'
 				);
 			} );
 
@@ -2346,7 +2364,7 @@ describe( 'DataFilter', () => {
 				} );
 
 				expect( editor.getData() ).to.equal(
-					'<section data-bar="baz bar" data-foo="bar baz"><p>foobar</p></section>'
+					'<section data-foo="bar baz" data-bar="baz bar"><p>foobar</p></section>'
 				);
 			} );
 

diff --git a/packages/ckeditor5-html-support/tests/integrations/mediaembed.js b/packages/ckeditor5-html-support/tests/integrations/mediaembed.js
@@ -1159,7 +1159,7 @@ describe( 'MediaEmbedElementSupport', () => {
 			} );
 
 			expect( editor.getData() ).to.equal(
-				'<p><oembed data-foo="foo" url="https://www.youtube.com/watch?v=ZVv7UMQPEWk"></oembed></p>'
+				'<p><oembed url="https://www.youtube.com/watch?v=ZVv7UMQPEWk" data-foo="foo"></oembed></p>'
 			);
 		} );
 
@@ -1278,7 +1278,7 @@ describe( 'MediaEmbedElementSupport', () => {
 
 			expect( editor.getData() ).to.equal(
 				'<figure class="media" data-foo="foo">' +
-					'<p><oembed data-foo="foo" url="https://www.youtube.com/watch?v=ZVv7UMQPEWk"></oembed></p>' +
+					'<p><oembed url="https://www.youtube.com/watch?v=ZVv7UMQPEWk" data-foo="foo"></oembed></p>' +
 				'</figure>'
 			);
 

diff --git a/packages/ckeditor5-html-support/tests/integrations/table.js b/packages/ckeditor5-html-support/tests/integrations/table.js
@@ -925,14 +925,14 @@ describe( 'TableElementSupport', () => {
 		expect( editor.getData() ).to.equalMarkup(
 			'<figure class="table">' +
 				'<table>' +
-					'<thead valign="bottom" lang="en" dir="ltr" align="right">' +
+					'<thead align="right" dir="ltr" lang="en" valign="bottom">' +
 						'<tr>' +
 							'<th>Bar</th>' +
 						'</tr>' +
 					'</thead>' +
-					'<tbody valign="bottom" lang="en" dir="ltr" align="right">' +
-						'<tr valign="bottom" align="right">' +
-							'<td valign="bottom" align="right">Foo</td>' +
+					'<tbody align="right" dir="ltr" lang="en" valign="bottom">' +
+						'<tr align="right" valign="bottom">' +
+							'<td align="right" valign="bottom">Foo</td>' +
 						'</tr>' +
 					'</tbody>' +
 				'</table>' +