Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #1184 from ckeditor/t/1179
Browse files Browse the repository at this point in the history 
Fix: View stringify utility now sorts CSS classes and values in `style` attribute. Closes #1179.
  • Loading branch information
@szymonkups
szymonkups authored Nov 20, 2017
2 parents 2924d52 + 7730795 commit fc7da80
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 33 deletions.
17 changes: 16 additions & 1 deletion src/dev-utils/view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,22 @@ class ViewStringify {
const keys = [ ...element.getAttributeKeys() ].sort();

for ( const attribute of keys ) {
attributes.push( `${ attribute }="${ element.getAttribute( attribute ) }"` );
let attributeValue;

if ( attribute === 'class' ) {
attributeValue = [ ...element.getClassNames() ]
.sort()
.join( ' ' );
} else if ( attribute === 'style' ) {
attributeValue = [ ...element.getStyleNames() ]
.sort()
.map( style => `${ style }:${ element.getStyle( style ) }` )
.join( ';' );
} else {
attributeValue = element.getAttribute( attribute );
}

attributes.push( `${ attribute }="${ attributeValue }"` );
}

return attributes.join( ' ' );
Expand Down
28 changes: 14 additions & 14 deletions tests/conversion/model-selection-to-view-converters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,9 @@ describe( 'model-selection-to-view-converters', () => {
beforeEach( () => {
function themeElementCreator( themeValue ) {
if ( themeValue == 'important' ) {
return new ViewAttributeElement( 'strong', { style: 'text-transform:uppercase;' } );
return new ViewAttributeElement( 'strong', { style: 'text-transform:uppercase' } );
} else if ( themeValue == 'gold' ) {
return new ViewAttributeElement( 'span', { style: 'color:yellow;' } );
return new ViewAttributeElement( 'span', { style: 'color:yellow' } );
}
}

Expand All @@ -545,31 +545,31 @@ describe( 'model-selection-to-view-converters', () => {
test(
[ 1, 5 ],
'fo<$text theme="gold">ob</$text>ar',
'f{o<span style="color:yellow;">ob</span>a}r'
'f{o<span style="color:yellow">ob</span>a}r'
);
} );

it( 'in same attribute', () => {
test(
[ 2, 4 ],
'f<$text theme="gold">ooba</$text>r',
'f<span style="color:yellow;">o{ob}a</span>r'
'f<span style="color:yellow">o{ob}a</span>r'
);
} );

it( 'in same attribute, selection same as attribute', () => {
test(
[ 2, 4 ],
'fo<$text theme="important">ob</$text>ar',
'fo{<strong style="text-transform:uppercase;">ob</strong>}ar'
'fo{<strong style="text-transform:uppercase">ob</strong>}ar'
);
} );

it( 'starts in attribute, ends in text node', () => {
test(
[ 3, 5 ],
'fo<$text theme="important">ob</$text>ar',
'fo<strong style="text-transform:uppercase;">o{b</strong>a}r'
'fo<strong style="text-transform:uppercase">o{b</strong>a}r'
);
} );
} );
Expand All @@ -579,15 +579,15 @@ describe( 'model-selection-to-view-converters', () => {
test(
[ 3, 3 ],
'f<$text theme="gold">ooba</$text>r',
'f<span style="color:yellow;">oo{}ba</span>r'
'f<span style="color:yellow">oo{}ba</span>r'
);
} );

it( 'in container with theme attribute', () => {
test(
[ 1, 1 ],
'foobar',
'f<strong style="text-transform:uppercase;">[]</strong>oobar',
'f<strong style="text-transform:uppercase">[]</strong>oobar',
{ theme: 'important' }
);
} );
Expand All @@ -596,9 +596,9 @@ describe( 'model-selection-to-view-converters', () => {
test(
[ 3, 3 ],
'f<$text theme="gold">ooba</$text>r',
'f<span style="color:yellow;">oo</span>' +
'<em><span style="color:yellow;">[]</span></em>' +
'<span style="color:yellow;">ba</span>r',
'f<span style="color:yellow">oo</span>' +
'<em><span style="color:yellow">[]</span></em>' +
'<span style="color:yellow">ba</span>r',
{ italic: true }
);
} );
Expand All @@ -611,9 +611,9 @@ describe( 'model-selection-to-view-converters', () => {
test(
[ 3, 3 ],
'f<$text theme="gold">ooba</$text>r',
'f<span style="color:yellow;">oo</span>' +
'<strong style="text-transform:uppercase;">[]</strong>' +
'<span style="color:yellow;">ba</span>r',
'f<span style="color:yellow">oo</span>' +
'<strong style="text-transform:uppercase">[]</strong>' +
'<span style="color:yellow">ba</span>r',
{ theme: 'important' }
);
} );
Expand Down
18 changes: 18 additions & 0 deletions tests/dev-utils/view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,24 @@ describe( 'view test utils', () => {
expect( stringify( p, null, { showType: true } ) )
.to.equal( '<container:p><ui:span></ui:span></container:p>' );
} );

it( 'should sort classes in specified element', () => {
const text = new Text( 'foobar' );
const b = new Element( 'b', {
class: 'zz xx aa'
}, text );

expect( stringify( b ) ).to.equal( '<b class="aa xx zz">foobar</b>' );
} );

it( 'should sort styles in specified element', () => {
const text = new Text( 'foobar' );
const i = new Element( 'i', {
style: 'text-decoration: underline; font-weight: bold'
}, text );

expect( stringify( i ) ).to.equal( '<i style="font-weight:bold;text-decoration:underline">foobar</i>' );
} );
} );

describe( 'parse', () => {
Expand Down
12 changes: 6 additions & 6 deletions tests/view/writer/unwrap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,17 +259,17 @@ describe( 'writer', () => {

it( 'should unwrap single element by removing matching classes', () => {
test(
'<container:p>[<attribute:b view-priority="1" class="foo bar baz">test</attribute:b>]</container:p>',
'<container:p>[<attribute:b view-priority="1" class="bar baz foo">test</attribute:b>]</container:p>',
'<attribute:b view-priority="1" class="baz foo"></attribute:b>',
'<container:p>[<attribute:b view-priority="1" class="bar">test</attribute:b>]</container:p>'
);
} );

it( 'should not unwrap single element when classes are different', () => {
test(
'<container:p>[<attribute:b view-priority="1" class="foo bar baz">test</attribute:b>]</container:p>',
'<container:p>[<attribute:b view-priority="1" class="bar baz foo">test</attribute:b>]</container:p>',
'<attribute:b view-priority="1" class="baz foo qux"></attribute:b>',
'<container:p>[<attribute:b view-priority="1" class="foo bar baz">test</attribute:b>]</container:p>'
'<container:p>[<attribute:b view-priority="1" class="bar baz foo">test</attribute:b>]</container:p>'
);
} );

Expand All @@ -279,18 +279,18 @@ describe( 'writer', () => {
'[<attribute:b view-priority="1" style="color:red;position:absolute;top:10px;">test</attribute:b>]' +
'</container:p>',
'<attribute:b view-priority="1" style="position: absolute;"></attribute:b>',
'<container:p>[<attribute:b view-priority="1" style="color:red;top:10px;">test</attribute:b>]</container:p>'
'<container:p>[<attribute:b view-priority="1" style="color:red;top:10px">test</attribute:b>]</container:p>'
);
} );

it( 'should not unwrap single element when styles are different', () => {
test(
'<container:p>' +
'[<attribute:b view-priority="1" style="color:red;position:absolute;top:10px;">test</attribute:b>]' +
'[<attribute:b view-priority="1" style="color:red;position:absolute;top:10px">test</attribute:b>]' +
'</container:p>',
'<attribute:b view-priority="1" style="position: relative;"></attribute:b>',
'<container:p>' +
'[<attribute:b view-priority="1" style="color:red;position:absolute;top:10px;">test</attribute:b>]' +
'[<attribute:b view-priority="1" style="color:red;position:absolute;top:10px">test</attribute:b>]' +
'</container:p>'
);
} );
Expand Down
24 changes: 12 additions & 12 deletions tests/view/writer/wrap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,26 +227,26 @@ describe( 'writer', () => {
test(
'<container:p>[<attribute:b view-priority="1" class="foo bar baz"></attribute:b>]</container:p>',
'<attribute:b view-priority="1" class="foo bar qux jax"></attribute:b>',
'<container:p>[<attribute:b view-priority="1" class="foo bar baz qux jax"></attribute:b>]</container:p>'
'<container:p>[<attribute:b view-priority="1" class="bar baz foo jax qux"></attribute:b>]</container:p>'
);
} );

it( 'should wrap single element by merging styles', () => {
test(
'<container:p>[<attribute:b view-priority="1" style="color:red; position: absolute;"></attribute:b>]</container:p>',
'<attribute:b view-priority="1" style="color:red; top: 20px;"></attribute:b>',
'<container:p>[<attribute:b view-priority="1" style="color:red;position:absolute;top:20px;"></attribute:b>]</container:p>'
'<container:p>[<attribute:b view-priority="1" style="color:red; position: absolute"></attribute:b>]</container:p>',
'<attribute:b view-priority="1" style="color:red; top: 20px"></attribute:b>',
'<container:p>[<attribute:b view-priority="1" style="color:red;position:absolute;top:20px"></attribute:b>]</container:p>'
);
} );

it( 'should not merge styles when they differ', () => {
test(
'<container:p>[<attribute:b view-priority="1" style="color:red;"></attribute:b>]</container:p>',
'<attribute:b view-priority="1" style="color:black;"></attribute:b>',
'<container:p>[<attribute:b view-priority="1" style="color:red"></attribute:b>]</container:p>',
'<attribute:b view-priority="1" style="color:black"></attribute:b>',
'<container:p>' +
'[' +
'<attribute:b view-priority="1" style="color:black;">' +
'<attribute:b view-priority="1" style="color:red;"></attribute:b>' +
'<attribute:b view-priority="1" style="color:black">' +
'<attribute:b view-priority="1" style="color:red"></attribute:b>' +
'</attribute:b>' +
']' +
'</container:p>'
Expand All @@ -255,12 +255,12 @@ describe( 'writer', () => {

it( 'should not merge single elements when they have different priority', () => {
test(
'<container:p>[<attribute:b view-priority="2" style="color:red;"></attribute:b>]</container:p>',
'<attribute:b view-priority="1" style="color:red;"></attribute:b>',
'<container:p>[<attribute:b view-priority="2" style="color:red"></attribute:b>]</container:p>',
'<attribute:b view-priority="1" style="color:red"></attribute:b>',
'<container:p>' +
'[' +
'<attribute:b view-priority="1" style="color:red;">' +
'<attribute:b view-priority="2" style="color:red;"></attribute:b>' +
'<attribute:b view-priority="1" style="color:red">' +
'<attribute:b view-priority="2" style="color:red"></attribute:b>' +
'</attribute:b>' +
']</container:p>'
);
Expand Down

0 comments on commit fc7da80

Please sign in to comment.