Skip to content

Commit

Permalink
Merge pull request #14334 from ckeditor/ck/14309-css-property-normali…
Browse files Browse the repository at this point in the history 
…zer-white-spaces

Fix (engine): The CSS shorthand value normalizer should work for values with white spaces. Closes #14309.
  • Loading branch information
@arkflpc
arkflpc committed Jun 6, 2023
2 parents a54ae4a + 7e38f25 commit c14cdff
Show file tree
Hide file tree
Showing 4 changed files with 193 additions and 10 deletions.
11 changes: 7 additions & 4 deletions packages/ckeditor5-engine/src/view/styles/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ const RGBA_COLOR_REGEXP = /^rgba\([ ]?([0-9]{1,3}[ %]?,[ ]?){3}(1|[0-9]+%|[0]?\.
const HSL_COLOR_REGEXP = /^hsl\([ ]?([0-9]{1,3}[ %]?[,]?[ ]*){3}(1|[0-9]+%|[0]?\.?[0-9]+)?\)$/i;
const HSLA_COLOR_REGEXP = /^hsla\([ ]?([0-9]{1,3}[ %]?,[ ]?){2,3}(1|[0-9]+%|[0]?\.?[0-9]+)\)$/i;

// Note: This regexp hardcodes a single level of nested () for values such as `calc( var( ...) + ...)`.
// If this gets more complex, a proper parser should be used instead.
const CSS_SHORTHAND_VALUE_REGEXP = /\w+\((?:[^()]|\([^()]*\))*\)|\S+/gi;

const COLOR_NAMES = new Set( [
// CSS Level 1
'black', 'silver', 'gray', 'white', 'maroon', 'red', 'purple', 'fuchsia',
Expand Down Expand Up @@ -249,8 +253,7 @@ export function getPositionShorthandNormalizer( shorthand: string ) {
* ```
*/
export function getShorthandValues( string: string ): Array<string> {
return string
.replace( /, /g, ',' ) // Exclude comma from spaces evaluation as values are separated by spaces.
.split( ' ' )
.map( string => string.replace( /,/g, ', ' ) ); // Restore original notation.
const matches = string.matchAll( CSS_SHORTHAND_VALUE_REGEXP );

return Array.from( matches ).map( i => i[ 0 ] );
}
50 changes: 46 additions & 4 deletions packages/ckeditor5-engine/tests/view/styles/border.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,22 @@ describe( 'Border styles normalization', () => {
} );
} );

it( 'should set all border colors (value with white spaces)', () => {
styles.setTo(
'border-color: rgb(10 , 10, 10 ) rgba(100, 100, 100, .3 )' +
' rgb( 20% 20% 20% ) rgba( 255 255 255 / .5 ) '
);

expect( styles.getNormalized( 'border' ) ).to.deep.equal( {
color: {
top: 'rgb(10 , 10, 10 )',
right: 'rgba(100, 100, 100, .3 )',
bottom: 'rgb( 20% 20% 20% )',
left: 'rgba( 255 255 255 / .5 )'
}
} );
} );

it( 'should merge with border shorthand', () => {
styles.setTo( 'border:1px solid blue;border-color:cyan black;' );

Expand Down Expand Up @@ -451,10 +467,10 @@ describe( 'Border styles normalization', () => {
styles.setTo( 'border:rgb(0, 30%,35);' );

expect( styles.getNormalized( 'border-color' ) ).to.deep.equal( {
top: 'rgb(0, 30%, 35)',
right: 'rgb(0, 30%, 35)',
bottom: 'rgb(0, 30%, 35)',
left: 'rgb(0, 30%, 35)'
top: 'rgb(0, 30%,35)',
right: 'rgb(0, 30%,35)',
bottom: 'rgb(0, 30%,35)',
left: 'rgb(0, 30%,35)'
} );
} );

Expand Down Expand Up @@ -523,6 +539,19 @@ describe( 'Border styles normalization', () => {
} );
} );

it( 'should set all border styles (value with white spaces)', () => {
styles.setTo( 'border-style: solid dotted var( --dashed ) ridge ;' );

expect( styles.getNormalized( 'border' ) ).to.deep.equal( {
style: {
top: 'solid',
right: 'dotted',
bottom: 'var( --dashed )',
left: 'ridge'
}
} );
} );

it( 'should parse none value', () => {
styles.setTo( 'border:none;' );

Expand Down Expand Up @@ -610,6 +639,19 @@ describe( 'Border styles normalization', () => {
} );
} );

it( 'should set all border widths (value with white spaces)', () => {
styles.setTo( 'border-width: 1px .34cm 90.1rem var(--foo) ;' );

expect( styles.getNormalized( 'border' ) ).to.deep.equal( {
width: {
top: '1px',
right: '.34cm',
bottom: '90.1rem',
left: 'var(--foo)'
}
} );
} );

it( 'should parse px value', () => {
styles.setTo( 'border:1px;' );

Expand Down
140 changes: 139 additions & 1 deletion packages/ckeditor5-engine/tests/view/styles/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,31 @@ describe( 'Styles utils', () => {
left: 'bar'
} );
} );

it( 'should work with values containing white spaces', () => {
expect( getBoxSidesValues(
' rgb(10 , 10, 10 ) rgba(100, 100, 100, .3 )' +
' rgb( 20% 20% 20% ) rgba( 255 255 255 / .5 ) '
) )
.to.deep.equal( {
top: 'rgb(10 , 10, 10 )',
right: 'rgba(100, 100, 100, .3 )',
bottom: 'rgb( 20% 20% 20% )',
left: 'rgba( 255 255 255 / .5 )'
} );
} );

it( 'should work with values containing nested declarations', () => {
expect( getBoxSidesValues( ' calc( 10px - 3px ) calc( var( --foo-var ) + 20px )' ) ).to.deep.equal( {
top: 'calc( 10px - 3px )',
right: 'calc( var( --foo-var ) + 20px )',
bottom: 'calc( 10px - 3px )',
left: 'calc( var( --foo-var ) + 20px )'
} );
} );
} );

describe( 'getParts()', () => {
describe( 'getShorthandValues()', () => {
it( 'should split string to separate values', () => {
expect( getShorthandValues( 'foo bar' ) ).to.deep.equal( [ 'foo', 'bar' ] );
} );
Expand All @@ -281,6 +303,122 @@ describe( 'Styles utils', () => {
expect( getShorthandValues( 'foo bar(1, 3, 5) url("example.com:foo/bar?q=b")' ) )
.to.deep.equal( [ 'foo', 'bar(1, 3, 5)', 'url("example.com:foo/bar?q=b")' ] );
} );

describe( 'for colors', () => {
it( 'should split color declarations: named colors', () => {
expect( getShorthandValues( 'red green black' ) ).to.deep.equal( [
'red', 'green', 'black'
] );
} );

it( 'should split color declarations: hex colors', () => {
expect( getShorthandValues( '#000 #000000EE' ) ).to.deep.equal( [
'#000', '#000000EE'
] );
} );

it( 'should split color declarations: rgb colors', () => {
expect( getShorthandValues( 'rgb(10, 10, 10) rgba(100, 100, 100, .3) rgb(20% 20% 20%) rgba(255 255 255 / .5)' ) )
.to.deep.equal( [
'rgb(10, 10, 10)', 'rgba(100, 100, 100, .3)', 'rgb(20% 20% 20%)', 'rgba(255 255 255 / .5)'
] );
} );

it( 'should split color declarations: hsl colors', () => {
expect( getShorthandValues( 'hsl(50 80% 40%) hsl(212.4, 89.3%, 89%) hsla(209, 90%, 72%,.3) hsla(0.3turn 60% 45% / .7)' ) )
.to.deep.equal( [
'hsl(50 80% 40%)', 'hsl(212.4, 89.3%, 89%)', 'hsla(209, 90%, 72%,.3)', 'hsla(0.3turn 60% 45% / .7)'
] );
} );

it( 'should split color declarations: other color formats', () => {
expect( getShorthandValues( 'hwb(50deg 30% 40%) cmyk(0 81% 81% 30%) color(xyz 22% 26% 53%) lab(30 59.4 -96)' ) )
.to.deep.equal( [
'hwb(50deg 30% 40%)', 'cmyk(0 81% 81% 30%)', 'color(xyz 22% 26% 53%)', 'lab(30 59.4 -96)'
] );
} );

describe( 'with additional white spaces in declarations', () => {
it( 'should split color declarations: named colors', () => {
expect( getShorthandValues( ' red green black ' ) ).to.deep.equal( [
'red', 'green', 'black'
] );
} );

it( 'should split color declarations: hex colors', () => {
expect( getShorthandValues( ' #000 #000000EE ' ) ).to.deep.equal( [
'#000', '#000000EE'
] );
} );

it( 'should split color declarations: rgb colors', () => {
expect( getShorthandValues(
' rgb(10 , 10, 10 ) rgba(100, 100, 100, .3 )' +
' rgb( 20% 20% 20% ) rgba( 255 255 255 / .5 ) '
) )
.to.deep.equal( [
'rgb(10 , 10, 10 )',
'rgba(100, 100, 100, .3 )',
'rgb( 20% 20% 20% )',
'rgba( 255 255 255 / .5 )'
] );
} );

it( 'should split color declarations: hsl colors', () => {
expect( getShorthandValues(
' hsl( 50 80% 40%) hsl( 212.4, 89.3%, 89% )' +
' hsla( 209, 90%, 72%, .3 ) hsla( 0.3turn 60% 45% / .7 )'
) )
.to.deep.equal( [
'hsl( 50 80% 40%)',
'hsl( 212.4, 89.3%, 89% )',
'hsla( 209, 90%, 72%, .3 )',
'hsla( 0.3turn 60% 45% / .7 )'
] );
} );

it( 'should split color declarations: other color formats', () => {
expect( getShorthandValues(
' hwb( 50deg 30% 40% ) cmyk( 0 81% 81% 30% )' +
' color( xyz 22% 26% 53%) lab( 30 59.4 -96 ) '
) )
.to.deep.equal( [
'hwb( 50deg 30% 40% )',
'cmyk( 0 81% 81% 30% )',
'color( xyz 22% 26% 53%)',
'lab( 30 59.4 -96 )'
] );
} );
} );
} );

describe( 'for non-color declarations', () => {
it( 'should split size declarations: simple units', () => {
expect( getShorthandValues( '3px 2em 10vw 20%' ) ).to.deep.equal( [
'3px', '2em', '10vw', '20%'
] );
} );

it( 'should split size declarations: values with calc() expressions', () => {
expect( getShorthandValues( 'calc(10px - 3px) calc(var(--foo-var) + 20px)' ) ).to.deep.equal( [
'calc(10px - 3px)', 'calc(var(--foo-var) + 20px)'
] );
} );

describe( 'with additional white spaces', () => {
it( 'should split size declarations: simple units', () => {
expect( getShorthandValues( ' 3px 2em 10vw 20% ' ) ).to.deep.equal( [
'3px', '2em', '10vw', '20%'
] );
} );

it( 'should split size declarations: values with calc() expressions', () => {
expect( getShorthandValues( ' calc( 10px - 3px ) calc( var( --foo-var ) + 20px )' ) ).to.deep.equal( [
'calc( 10px - 3px )', 'calc( var( --foo-var ) + 20px )'
] );
} );
} );
} );
} );

function testValues( values, callback ) {
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-table/tests/tablecellproperties/tablecellpropertiesediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ describe( 'table cell properties', () => {

const tableCell = model.document.getRoot().getNodeByPath( [ 0, 0, 0 ] );

assertTRBLAttribute( tableCell, 'tableCellBorderColor', '#f00', null, 'rgba(255, 0, 0, 1)', null );
assertTRBLAttribute( tableCell, 'tableCellBorderColor', '#f00', null, 'rgba(255,0,0,1)', null );
assertTRBLAttribute( tableCell, 'tableCellBorderStyle', 'solid', null, 'ridge', null );
assertTRBLAttribute( tableCell, 'tableCellBorderWidth', '1px', null, '2em', null );
} );
Expand Down

0 comments on commit c14cdff

Please sign in to comment.