I/3287: Styles normalization (part of table styles)

src/conversion/viewconsumable.js

@@ -10,6 +10,43 @@
 import { isArray } from 'lodash-es';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
+const styleConsumablesMap = new Map();
+
+addStyleConsumeAlso( 'margin', [ 'margin-top', 'margin-right', 'margin-bottom', 'margin-left' ] );
+addStyleConsumeAlso( 'padding', [ 'padding-top', 'padding-right', 'padding-bottom', 'padding-left' ] );
+
+addStyleConsumeAlso( 'border', [ 'border-color', 'border-style', 'border-width' ] );
+addStyleConsumeAlso( 'border', [ 'border-top', 'border-right', 'border-bottom', 'border-left' ] );
+addStyleConsumeAlso( 'border', [
+	'border-top-color', 'border-right-color', 'border-bottom-color', 'border-left-color',
+	'border-top-style', 'border-right-style', 'border-bottom-style', 'border-left-style',
+	'border-top-width', 'border-right-width', 'border-bottom-width', 'border-left-width'
+] );
+addStyleConsumeAlso( 'border-color', [ 'border-top-color', 'border-right-color', 'border-bottom-color', 'border-left-color' ] );
+addStyleConsumeAlso( 'border-style', [ 'border-top-style', 'border-right-style', 'border-bottom-style', 'border-left-style' ] );
+addStyleConsumeAlso( 'border-width', [ 'border-top-width', 'border-right-width', 'border-bottom-width', 'border-left-width' ] );
+
+addStyleConsumeAlso( 'border-top', [ 'border-top-color', 'border-top-style', 'border-top-width' ] );
+addStyleConsumeAlso( 'border-right', [ 'border-right-color', 'border-right-style', 'border-right-width' ] );
+addStyleConsumeAlso( 'border-bottom', [ 'border-bottom-color', 'border-bottom-style', 'border-bottom-width' ] );
+addStyleConsumeAlso( 'border-left', [ 'border-left-color', 'border-left-style', 'border-left-width' ] );
+
+function addStyleConsumeAlso( shorthandName, also ) {
+	addConsumable( shorthandName, also );
+
+	for ( const alsoName of also ) {
+		addConsumable( alsoName, [ shorthandName ] );
+	}
+
+	function addConsumable( name, values ) {
+		if ( !styleConsumablesMap.has( name ) ) {
+			styleConsumablesMap.set( name, [] );
+		}
+
+		styleConsumablesMap.get( name ).push( ...values );
+	}
+}
+
 /**
  * Class used for handling consumption of view {@link module:engine/view/element~Element elements},
  * {@link module:engine/view/text~Text text nodes} and {@link module:engine/view/documentfragment~DocumentFragment document fragments}.
@@ -507,6 +544,12 @@ class ViewElementConsumables {
 			}
 
 			consumables.set( name, true );
+
+			if ( type === 'styles' && styleConsumablesMap.has( name ) ) {
+				for ( const alsoName of styleConsumablesMap.get( name ) ) {
+					consumables.set( alsoName, true );
+				}
+			}
 		}
 	}
 
@@ -568,6 +611,12 @@ class ViewElementConsumables {
 				this._consume( consumableName, [ ...this._consumables[ consumableName ].keys() ] );
 			} else {
 				consumables.set( name, false );
+
+				if ( styleConsumablesMap.has( name ) ) {
+					for ( const toConsume of styleConsumablesMap.get( name ) ) {
+						consumables.set( toConsume, false );
+					}
+				}
 			}
 		}
 	}

src/view/element.js

@@ -14,6 +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 Styles from './styles';
 
 // @if CK_DEBUG_ENGINE // const { convertMapToTags } = require( '../dev-utils/utils' );
 
@@ -105,16 +106,17 @@ 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._styles.setStyle( this._attrs.get( 'style' ) );
+
 			this._attrs.delete( 'style' );
 		}
 
@@ -264,17 +266,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 );
@@ -342,8 +334,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 this._styles.getStyleNames() ) {
+			if (
+				!otherElement._styles.hasProperty( property ) ||
+				otherElement._styles.getInlineProperty( property ) !== this._styles.getInlineProperty( property )
+			) {
 				return false;
 			}
 		}
@@ -384,10 +379,14 @@ export default class Element extends Node {
 	 * Undefined is returned if style does not exist.
 	 *
 	 * @param {String} property
-	 * @returns {String|undefined}
+	 * @returns {String|Object|undefined}
 	 */
 	getStyle( property ) {
-		return this._styles.get( property );
+		return this._styles.getInlineProperty( property );
+	}
+
+	getNormalizedStyle( property ) {
+		return this._styles.getNormalized( property );
 	}
 
 	/**
@@ -396,7 +395,7 @@ export default class Element extends Node {
 	 * @returns {Iterable.<String>}
 	 */
 	getStyleNames() {
-		return this._styles.keys();
+		return this._styles.getStyleNames();
 	}
 
 	/**
@@ -410,7 +409,7 @@ export default class Element extends Node {
 	 */
 	hasStyle( ...property ) {
 		for ( const name of property ) {
-			if ( !this._styles.has( name ) ) {
+			if ( !this._styles.hasProperty( name ) ) {
 				return false;
 			}
 		}
@@ -487,12 +486,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 }` );
 	}
 
@@ -519,7 +518,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.insertProperty( this._styles.getNormalized() );
 
 		// Clone custom properties.
 		cloned._customProperties = new Map( this._customProperties );
@@ -616,7 +615,7 @@ export default class Element extends Node {
 		if ( key == 'class' ) {
 			parseClasses( this._classes, value );
 		} else if ( key == 'style' ) {
-			parseInlineStyles( this._styles, value );
+			this._styles.setStyle( value );
 		} else {
 			this._attrs.set( key, value );
 		}
@@ -647,7 +646,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;
@@ -714,15 +713,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.insertProperty( property, value );
 	}
 
 	/**
@@ -740,7 +731,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.removeProperty( name ) );
 	}
 
 	/**
@@ -826,82 +817,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.
-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.
 //