Selaa lähdekoodia

Rewrite Element class to use Styles normalization class.

Maciej Gołaszewski 6 vuotta sitten
vanhempi
commit
59cab27449

+ 21 - 113
packages/ckeditor5-engine/src/view/element.js

@@ -14,7 +14,7 @@ import objectToMap from '@ckeditor/ckeditor5-utils/src/objecttomap';
 import isIterable from '@ckeditor/ckeditor5-utils/src/isiterable';
 import Matcher from './matcher';
 import { isPlainObject } from 'lodash-es';
-import { getStyleProxy } from './stylenormalizer';
+import Styles from './styles';
 
 /**
  * View element.
@@ -104,17 +104,16 @@ export default class Element extends Node {
 		}
 
 		/**
-		 * Map of styles.
+		 * Normalized styles.
 		 *
 		 * @protected
 		 * @member {Map} module:engine/view/element~Element#_styles
 		 */
-		this._styles = new Map();
+		this._styles = new Styles();
 
 		if ( this._attrs.has( 'style' ) ) {
 			// Remove style attribute and handle it by styles map.
-			parseInlineStyles( this._styles, this._attrs.get( 'style' ) );
-			this._stylesProxy = getStyleProxy( this._attrs.get( 'style' ) );
+			this._styles.setStyle( this._attrs.get( 'style' ) );
 
 			this._attrs.delete( 'style' );
 		}
@@ -265,17 +264,7 @@ export default class Element extends Node {
 		}
 
 		if ( key == 'style' ) {
-			if ( this._styles.size > 0 ) {
-				let styleString = '';
-
-				for ( const [ property, value ] of this._styles ) {
-					styleString += `${ property }:${ value };`;
-				}
-
-				return styleString;
-			}
-
-			return undefined;
+			return this._styles.getInlineStyle();
 		}
 
 		return this._attrs.get( key );
@@ -343,8 +332,11 @@ export default class Element extends Node {
 		}
 
 		// Check if styles are the same.
-		for ( const [ property, value ] of this._styles ) {
-			if ( !otherElement._styles.has( property ) || otherElement._styles.get( property ) !== value ) {
+		for ( const property of Object.keys( this._styles._styles ) ) {
+			if (
+				!otherElement._styles.hasRule( property ) ||
+				otherElement._styles.getInlineRule( property ) !== this._styles.getInlineRule( property )
+			) {
 				return false;
 			}
 		}
@@ -388,7 +380,7 @@ export default class Element extends Node {
 	 * @returns {String|undefined}
 	 */
 	getStyle( property ) {
-		return this._styles.get( property );
+		return this._styles.getInlineRule( property );
 	}
 
 	/**
@@ -397,7 +389,7 @@ export default class Element extends Node {
 	 * @returns {Iterable.<String>}
 	 */
 	getStyleNames() {
-		return this._styles.keys();
+		return Object.keys( this._styles._styles );
 	}
 
 	/**
@@ -411,7 +403,7 @@ export default class Element extends Node {
 	 */
 	hasStyle( ...property ) {
 		for ( const name of property ) {
-			if ( !this._styles.has( name ) ) {
+			if ( !this._styles.hasRule( name ) ) {
 				return false;
 			}
 		}
@@ -488,12 +480,12 @@ export default class Element extends Node {
 	 */
 	getIdentity() {
 		const classes = Array.from( this._classes ).sort().join( ',' );
-		const styles = Array.from( this._styles ).map( i => `${ i[ 0 ] }:${ i[ 1 ] }` ).sort().join( ';' );
+		const styles = this._styles.getInlineStyle();
 		const attributes = Array.from( this._attrs ).map( i => `${ i[ 0 ] }="${ i[ 1 ] }"` ).sort().join( ' ' );
 
 		return this.name +
 			( classes == '' ? '' : ` class="${ classes }"` ) +
-			( styles == '' ? '' : ` style="${ styles }"` ) +
+			( !styles ? '' : ` style="${ styles }"` ) +
 			( attributes == '' ? '' : ` ${ attributes }` );
 	}
 
@@ -520,7 +512,7 @@ export default class Element extends Node {
 		// Classes and styles are cloned separately - this solution is faster than adding them back to attributes and
 		// parse once again in constructor.
 		cloned._classes = new Set( this._classes );
-		cloned._styles = new Map( this._styles );
+		cloned._styles.setStyle( this._styles.getInlineStyle() );
 
 		// Clone custom properties.
 		cloned._customProperties = new Map( this._customProperties );
@@ -617,8 +609,8 @@ export default class Element extends Node {
 		if ( key == 'class' ) {
 			parseClasses( this._classes, value );
 		} else if ( key == 'style' ) {
-			parseInlineStyles( this._styles, value );
-			this._stylesProxy = getStyleProxy( value );
+			// parseInlineStyles( this._styles, value );
+			this._styles.setStyle( value );
 		} else {
 			this._attrs.set( key, value );
 		}
@@ -649,7 +641,7 @@ export default class Element extends Node {
 
 		// Remove style attribute.
 		if ( key == 'style' ) {
-			if ( this._styles.size > 0 ) {
+			if ( this._styles.size ) {
 				this._styles.clear();
 
 				return true;
@@ -716,15 +708,7 @@ export default class Element extends Node {
 	_setStyle( property, value ) {
 		this._fireChange( 'attributes', this );
 
-		if ( isPlainObject( property ) ) {
-			const keys = Object.keys( property );
-
-			for ( const key of keys ) {
-				this._styles.set( key, property[ key ] );
-			}
-		} else {
-			this._styles.set( property, value );
-		}
+		this._styles.insertRule( property, value );
 	}
 
 	/**
@@ -742,7 +726,7 @@ export default class Element extends Node {
 		this._fireChange( 'attributes', this );
 
 		property = Array.isArray( property ) ? property : [ property ];
-		property.forEach( name => this._styles.delete( name ) );
+		property.forEach( name => this._styles.removeRule( name ) );
 	}
 
 	/**
@@ -802,82 +786,6 @@ function parseAttributes( attrs ) {
 	return attrs;
 }
 
-// Parses inline styles and puts property - value pairs into styles map.
-// Styles map is cleared before insertion.
-//
-// @param {Map.<String, String>} stylesMap Map to insert parsed properties and values.
-// @param {String} stylesString Styles to parse.
-export function parseInlineStyles( stylesMap, stylesString ) {
-	// `null` if no quote was found in input string or last found quote was a closing quote. See below.
-	let quoteType = null;
-	let propertyNameStart = 0;
-	let propertyValueStart = 0;
-	let propertyName = null;
-
-	stylesMap.clear();
-
-	// Do not set anything if input string is empty.
-	if ( stylesString === '' ) {
-		return;
-	}
-
-	// Fix inline styles that do not end with `;` so they are compatible with algorithm below.
-	if ( stylesString.charAt( stylesString.length - 1 ) != ';' ) {
-		stylesString = stylesString + ';';
-	}
-
-	// Seek the whole string for "special characters".
-	for ( let i = 0; i < stylesString.length; i++ ) {
-		const char = stylesString.charAt( i );
-
-		if ( quoteType === null ) {
-			// No quote found yet or last found quote was a closing quote.
-			switch ( char ) {
-				case ':':
-					// Most of time colon means that property name just ended.
-					// Sometimes however `:` is found inside property value (for example in background image url).
-					if ( !propertyName ) {
-						// Treat this as end of property only if property name is not already saved.
-						// Save property name.
-						propertyName = stylesString.substr( propertyNameStart, i - propertyNameStart );
-						// Save this point as the start of property value.
-						propertyValueStart = i + 1;
-					}
-
-					break;
-
-				case '"':
-				case '\'':
-					// Opening quote found (this is an opening quote, because `quoteType` is `null`).
-					quoteType = char;
-
-					break;
-
-				case ';': {
-					// Property value just ended.
-					// Use previously stored property value start to obtain property value.
-					const propertyValue = stylesString.substr( propertyValueStart, i - propertyValueStart );
-
-					if ( propertyName ) {
-						// Save parsed part.
-						stylesMap.set( propertyName.trim(), propertyValue.trim() );
-					}
-
-					propertyName = null;
-
-					// Save this point as property name start. Property name starts immediately after previous property value ends.
-					propertyNameStart = i + 1;
-
-					break;
-				}
-			}
-		} else if ( char === quoteType ) {
-			// If a quote char is found and it is a closing quote, mark this fact by `null`-ing `quoteType`.
-			quoteType = null;
-		}
-	}
-}
-
 // Parses class attribute and puts all classes into classes set.
 // Classes set s cleared before insertion.
 //

+ 113 - 27
packages/ckeditor5-engine/src/view/stylenormalizer.js

@@ -2,48 +2,52 @@
  * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
-import { parseInlineStyles } from './element';
 
-import { get, has, isObject, unset } from 'lodash-es';
+/**
+ * @module engine/view/styles
+ */
 
-export function getStyleProxy( styleString ) {
-	return new StyleProxy( styleString );
-}
+import { get, has, isObject, isPlainObject, unset } from 'lodash-es';
 
-// <-- FROM VIEW/MODEL
-// proxy.setStyle( 'border: 1px solid blue;' )
-//
-// <-> MODIFY
-// proxy.insertRule( 'border-top', '1px solid blue' ); // obj?
-// proxy.removeRule( 'border-top' );
-// proxy.clear();
-//
-// --> TO MODEL
-// proxy.getModel(); // full
-// proxy.getModel( 'border' );
-// proxy.getModel( 'border-top' );
-//
-// --> TO VIEW
-// proxy.getInlineStyle();
-// proxy.getInlineRule( 'border' );
-// proxy.getInlineRule( 'border-top' );
-export class StyleProxy {
+/**
+ * Styles class.
+ *
+ * Handles styles normalization.
+ */
+export default class Styles {
 	constructor( styleString = '' ) {
 		this._styles = {};
 
 		this.setStyle( styleString );
 	}
 
+	get size() {
+		return Object.keys( this._styles ).length;
+	}
+
 	setStyle( styleString = '' ) {
 		this._styles = parseStyle( styleString );
 	}
 
+	hasRule( name ) {
+		const nameNorm = name.replace( '-', '.' );
+
+		return has( this._styles, nameNorm ) || !!this._styles[ name ];
+	}
+
 	insertRule( nameOrObject, value ) {
-		parseRule( nameOrObject, value, this._styles );
+		if ( isPlainObject( nameOrObject ) ) {
+			for ( const key of Object.keys( nameOrObject ) ) {
+				this.insertRule( key, nameOrObject[ key ] );
+			}
+		} else {
+			parseRule( nameOrObject, value, this._styles );
+		}
 	}
 
 	removeRule( name ) {
 		unset( this._styles, name.replace( '-', '.' ) );
+		delete this._styles[ name ];
 	}
 
 	getModel( name ) {
@@ -61,13 +65,19 @@ export class StyleProxy {
 	getInlineStyle() {
 		const parsed = [];
 
-		for ( const key of Object.keys( this._styles ) ) {
+		const keys = Object.keys( this._styles ).sort();
+
+		if ( !keys.length ) {
+			return;
+		}
+
+		for ( const key of keys ) {
 			const model = this.getModel( key );
 
 			parsed.push( toInlineStyle( key, model ) );
 		}
 
-		return parsed.join( ';' ) + ( parsed.length ? ';' : '' );
+		return parsed.join( ';' );
 	}
 
 	getInlineRule( name ) {
@@ -94,7 +104,7 @@ export class StyleProxy {
 
 const borderPositionRegExp = /border-(top|right|bottom|left)$/;
 
-export function parseStyle( string, styleObject = {} ) {
+function parseStyle( string, styleObject = {} ) {
 	const map = new Map();
 
 	parseInlineStyles( map, string );
@@ -231,3 +241,79 @@ function toInlineBorder( object = {} ) {
 
 	return style.join( ' ' );
 }
+
+// Parses inline styles and puts property - value pairs into styles map.
+// Styles map is cleared before insertion.
+//
+// @param {Map.<String, String>} stylesMap Map to insert parsed properties and values.
+// @param {String} stylesString Styles to parse.
+function parseInlineStyles( stylesMap, stylesString ) {
+	// `null` if no quote was found in input string or last found quote was a closing quote. See below.
+	let quoteType = null;
+	let propertyNameStart = 0;
+	let propertyValueStart = 0;
+	let propertyName = null;
+
+	stylesMap.clear();
+
+	// Do not set anything if input string is empty.
+	if ( stylesString === '' ) {
+		return;
+	}
+
+	// Fix inline styles that do not end with `;` so they are compatible with algorithm below.
+	if ( stylesString.charAt( stylesString.length - 1 ) != ';' ) {
+		stylesString = stylesString + ';';
+	}
+
+	// Seek the whole string for "special characters".
+	for ( let i = 0; i < stylesString.length; i++ ) {
+		const char = stylesString.charAt( i );
+
+		if ( quoteType === null ) {
+			// No quote found yet or last found quote was a closing quote.
+			switch ( char ) {
+				case ':':
+					// Most of time colon means that property name just ended.
+					// Sometimes however `:` is found inside property value (for example in background image url).
+					if ( !propertyName ) {
+						// Treat this as end of property only if property name is not already saved.
+						// Save property name.
+						propertyName = stylesString.substr( propertyNameStart, i - propertyNameStart );
+						// Save this point as the start of property value.
+						propertyValueStart = i + 1;
+					}
+
+					break;
+
+				case '"':
+				case '\'':
+					// Opening quote found (this is an opening quote, because `quoteType` is `null`).
+					quoteType = char;
+
+					break;
+
+				case ';': {
+					// Property value just ended.
+					// Use previously stored property value start to obtain property value.
+					const propertyValue = stylesString.substr( propertyValueStart, i - propertyValueStart );
+
+					if ( propertyName ) {
+						// Save parsed part.
+						stylesMap.set( propertyName.trim(), propertyValue.trim() );
+					}
+
+					propertyName = null;
+
+					// Save this point as property name start. Property name starts immediately after previous property value ends.
+					propertyNameStart = i + 1;
+
+					break;
+				}
+			}
+		} else if ( char === quoteType ) {
+			// If a quote char is found and it is a closing quote, mark this fact by `null`-ing `quoteType`.
+			quoteType = null;
+		}
+	}
+}

+ 17 - 19
packages/ckeditor5-engine/tests/view/element.js

@@ -68,17 +68,18 @@ describe( 'Element', () => {
 			expect( el._classes.has( 'three' ) ).to.be.true;
 		} );
 
-		it( 'should move style attribute to style map', () => {
+		it( 'should move style attribute to style proxy', () => {
 			const el = new Element( 'p', { id: 'test', style: 'one: style1; two:style2 ; three : url(http://ckeditor.com)' } );
 
 			expect( el._attrs.has( 'style' ) ).to.be.false;
 			expect( el._attrs.has( 'id' ) ).to.be.true;
-			expect( el._styles.has( 'one' ) ).to.be.true;
-			expect( el._styles.get( 'one' ) ).to.equal( 'style1' );
-			expect( el._styles.has( 'two' ) ).to.be.true;
-			expect( el._styles.get( 'two' ) ).to.equal( 'style2' );
-			expect( el._styles.has( 'three' ) ).to.be.true;
-			expect( el._styles.get( 'three' ) ).to.equal( 'url(http://ckeditor.com)' );
+
+			expect( el._styles.hasRule( 'one' ) ).to.be.true;
+			expect( el._styles.getInlineRule( 'one' ) ).to.equal( 'style1' );
+			expect( el._styles.hasRule( 'two' ) ).to.be.true;
+			expect( el._styles.getInlineRule( 'two' ) ).to.equal( 'style2' );
+			expect( el._styles.hasRule( 'three' ) ).to.be.true;
+			expect( el._styles.getInlineRule( 'three' ) ).to.equal( 'url(http://ckeditor.com)' );
 		} );
 	} );
 
@@ -199,10 +200,10 @@ describe( 'Element', () => {
 
 			expect( clone ).to.not.equal( el );
 			expect( clone.name ).to.equal( el.name );
-			expect( clone._styles.has( 'color' ) ).to.be.true;
-			expect( clone._styles.get( 'color' ) ).to.equal( 'red' );
-			expect( clone._styles.has( 'font-size' ) ).to.be.true;
-			expect( clone._styles.get( 'font-size' ) ).to.equal( '12px' );
+			expect( clone._styles.hasRule( 'color' ) ).to.be.true;
+			expect( clone._styles.getInlineRule( 'color' ) ).to.equal( 'red' );
+			expect( clone._styles.hasRule( 'font-size' ) ).to.be.true;
+			expect( clone._styles.getInlineRule( 'font-size' ) ).to.equal( '12px' );
 		} );
 
 		it( 'should clone custom properties', () => {
@@ -515,7 +516,7 @@ describe( 'Element', () => {
 				el._setStyle( 'color', 'red' );
 				el._setStyle( 'top', '10px' );
 
-				expect( el.getAttribute( 'style' ) ).to.equal( 'color:red;top:10px;' );
+				expect( el.getAttribute( 'style' ) ).to.equal( 'color:red;top:10px' );
 			} );
 
 			it( 'should return undefined if no style attribute', () => {
@@ -538,7 +539,7 @@ describe( 'Element', () => {
 				el._setStyle( 'font-weight', 'bold' );
 
 				expect( Array.from( el.getAttributes() ) ).to.deep.equal( [
-					[ 'class', 'abc xyz' ], [ 'style', 'width:20px;font-weight:bold;' ]
+					[ 'class', 'abc xyz' ], [ 'style', 'font-weight:bold;width:20px' ]
 				] );
 			} );
 		} );
@@ -766,8 +767,7 @@ describe( 'Element', () => {
 			it( 'should set element style', () => {
 				el._setStyle( 'color', 'red' );
 
-				expect( el._styles.has( 'color' ) ).to.be.true;
-				expect( el._styles.get( 'color' ) ).to.equal( 'red' );
+				expect( el._styles._styles.color ).to.equal( 'red' );
 			} );
 
 			it( 'should fire change event with attributes type', done => {
@@ -785,10 +785,8 @@ describe( 'Element', () => {
 					position: 'fixed'
 				} );
 
-				expect( el._styles.has( 'color' ) ).to.be.true;
-				expect( el._styles.has( 'position' ) ).to.be.true;
-				expect( el._styles.get( 'color' ) ).to.equal( 'red' );
-				expect( el._styles.get( 'position' ) ).to.equal( 'fixed' );
+				expect( el._styles._styles.color ).to.equal( 'red' );
+				expect( el._styles._styles.position ).to.equal( 'fixed' );
 			} );
 		} );
 

+ 7 - 7
packages/ckeditor5-engine/tests/view/stylenormalizer.js

@@ -2,9 +2,9 @@
  * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
-import { StyleProxy } from '../../src/view/stylenormalizer';
+import StyleProxy from '../../src/view/styles';
 
-describe( 'Style proxy', () => {
+describe( 'Styles', () => {
 	let styleProxy;
 
 	beforeEach( () => {
@@ -68,7 +68,7 @@ describe( 'Style proxy', () => {
 	it( 'should output', () => {
 		styleProxy.setStyle( 'border:1px solid blue;' );
 
-		expect( styleProxy.getInlineStyle() ).to.equal( 'border:1px solid blue;' );
+		expect( styleProxy.getInlineStyle() ).to.equal( 'border:1px solid blue' );
 		expect( styleProxy.getInlineRule( 'border' ) ).to.equal( '1px solid blue' );
 		expect( styleProxy.getInlineRule( 'border-top' ) ).to.equal( '1px solid blue' );
 		expect( styleProxy.getInlineRule( 'border-right' ) ).to.equal( '1px solid blue' );
@@ -80,7 +80,7 @@ describe( 'Style proxy', () => {
 		styleProxy.setStyle( 'border:1px solid blue;border-left:#665511 dashed 2.7em;border-top:7px dotted #ccc;' );
 
 		expect( styleProxy.getInlineStyle() ).to.equal(
-			'border-top:7px dotted #ccc;border-right:1px solid blue;border-bottom:1px solid blue;border-left:2.7em dashed #665511;'
+			'border-top:7px dotted #ccc;border-right:1px solid blue;border-bottom:1px solid blue;border-left:2.7em dashed #665511'
 		);
 		expect( styleProxy.getInlineRule( 'border' ) ).to.be.undefined;
 		expect( styleProxy.getInlineRule( 'border-top' ) ).to.equal( '7px dotted #ccc' );
@@ -95,7 +95,7 @@ describe( 'Style proxy', () => {
 		styleProxy.insertRule( 'border-top', '7px dotted #ccc' );
 
 		expect( styleProxy.getInlineStyle() ).to.equal(
-			'border-top:7px dotted #ccc;border-right:1px solid blue;border-bottom:1px solid blue;border-left:2.7em dashed #665511;'
+			'border-top:7px dotted #ccc;border-right:1px solid blue;border-bottom:1px solid blue;border-left:2.7em dashed #665511'
 		);
 		expect( styleProxy.getInlineRule( 'border' ) ).to.be.undefined;
 		expect( styleProxy.getInlineRule( 'border-top' ) ).to.equal( '7px dotted #ccc' );
@@ -109,7 +109,7 @@ describe( 'Style proxy', () => {
 		styleProxy.removeRule( 'border-top' );
 
 		expect( styleProxy.getInlineStyle() ).to.equal(
-			'border-right:1px solid blue;border-bottom:1px solid blue;border-left:1px solid blue;'
+			'border-right:1px solid blue;border-bottom:1px solid blue;border-left:1px solid blue'
 		);
 		expect( styleProxy.getInlineRule( 'border' ) ).to.be.undefined;
 		expect( styleProxy.getInlineRule( 'border-top' ) ).to.be.undefined;
@@ -121,7 +121,7 @@ describe( 'Style proxy', () => {
 	it( 'pass-through', () => {
 		styleProxy.setStyle( 'foo-bar:baz 1px abc;margin: 2px 3em;' );
 
-		expect( styleProxy.getInlineStyle() ).to.equal( 'foo-bar:baz 1px abc;margin:2px 3em;' );
+		expect( styleProxy.getInlineStyle() ).to.equal( 'foo-bar:baz 1px abc;margin:2px 3em' );
 		expect( styleProxy.getInlineRule( 'foo-bar' ) ).to.equal( 'baz 1px abc' );
 		expect( styleProxy.getInlineRule( 'margin' ) ).to.equal( '2px 3em' );
 	} );

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

@@ -508,8 +508,8 @@ describe( 'UpcastWriter', () => {
 
 			writer.removeStyle( [ 'color', 'position' ], el );
 
-			expect( el.hasStyle( 'color' ) ).to.false;
-			expect( el.hasStyle( 'position' ) ).to.false;
+			expect( el.hasStyle( 'color' ) ).to.be.false;
+			expect( el.hasStyle( 'position' ) ).to.be.false;
 			expect( Array.from( el.getStyleNames() ).length ).to.equal( 0 );
 		} );