ソースを参照

Added validation for style attribute value.

Oskar Wrobel 9 年 前
コミット
d898bf66d6
2 ファイル変更157 行追加75 行削除
  1. 61 22
      packages/ckeditor5-ui/src/template.js
  2. 96 53
      packages/ckeditor5-ui/tests/template.js

+ 61 - 22
packages/ckeditor5-ui/src/template.js

@@ -406,19 +406,9 @@ export default class Template {
 			attrNs = attrValue[ 0 ].ns || null;
 
 			// Attribute style might have specific format so needs to be parsed in a specific way.
-			// 		input: { style: { width: '100px', height: '200px' } }
-			// 		output: style="width:100px;height:200px"
 			if ( attrName == 'style' ) {
-				const styleValue = attrValue[ 0 ].value || attrValue[ 0 ];
-
-				// If whole attribute is binded we don't parse it.
-				// It means that style will be render the same way as the rest of attributes:
-				// 		input: { style: 'width:100px;height:200px' }
-				// 		output: style="width:100px;height:200px"
-				if ( !styleValue.observable ) {
-					this._renderAttributeStyle( styleValue, el );
-					continue;
-				}
+				this._renderAttributeStyle( attrValue[ 0 ].value || attrValue[ 0 ], el );
+				continue;
 			}
 
 			// Activate binding if one is found. Cases:
@@ -455,34 +445,83 @@ export default class Template {
 
 	/**
 	 * Render attribute `style`.
-	 * Attribute style has specific format so needs to be parsed in a specific way. For example:
 	 *
-	 * 		style: {
-	 * 			property: value,
-	 * 			otherProperty: otherValue
+	 * Attribute style value could be an Object with static or binded to model properties:
+	 *
+	 *		new Model( {
+	 *			modelProperty: 'value'
+	 *		} );
+	 *
+	 *		attributes: {
+	 * 			style: {
+	 * 				property: value,
+	 * 				otherProperty: bind.to( 'modelProperty' ),
+	 * 			}
 	 * 		}
 	 *
-	 * 	will be rendered as:
+	 * 	or a String if whole style attribute is binded to model property:
 	 *
-	 * 		style="property:value;other-property:otherValue"
+	 * 		new Model( {
+	 *			style: 'value'
+	 *		} );
+	 *
+	 * 		attributes: {
+	 *			style: bind.to( 'style' )
+	 * 		}
+	 *
+	 * Note: Multiple-word properties are always defined in camelCase format:
+	 *
+	 * 		attributes: {
+	 * 			style: {
+	 * 				backgroundColor: ...,
+	 * 				borderWidth: ...
+	 * 			}
+	 * 		}
 	 *
-	 * Note: Multiple-word properties are always defined in camelCase format, and then are parsed to dash format.
 	 * Note: Attribute `style` is rendered without setting namespace because setting custom namespace seems to be not
 	 * necessary in this case.
 	 *
 	 * @private
-	 * @param {ui.TemplateDefinition.attributes.styles} styles Styles definition. Multiple-word properties needs to be
-	 * defined in camelCase format.
+	 * @param {ui.TemplateDefinition.attributes.styles} styles Styles definition.
 	 * @param {HTMLElement} el Element which is rendered.
 	 */
 	_renderAttributeStyle( styles, el ) {
+		// If whole attribute is binded we don't parse value.
+		if ( styles.observable ) {
+			const { emitter, observable, attribute } = styles;
+
+			// Validate value format for initial value.
+			if ( typeof observable[ attribute ] != 'string' ) {
+				throw new CKEditorError(
+					'template-renderAttributeStyle-invalid-format: Value for Whole binded property must be a string',
+					{ value: observable[ attribute ] }
+				);
+			}
+
+			emitter.listenTo( observable, `change:${ attribute }`, ( eventInfo, property, value ) => {
+				// Validate value format for updated value.
+				if ( typeof value != 'string' ) {
+					throw new CKEditorError(
+						'template-renderAttributeStyle-invalid-format: Value for Whole binded property must be a string',
+						{ value }
+					);
+				}
+
+				el.style.cssText = value;
+			} );
+
+			el.style.cssText = observable[ attribute ];
+
+			return;
+		}
+
 		// Iterate through every single style.
 		const initialStyles = Object.keys( styles ).map( ( style ) => {
 			// If style value is not observable.
 			if ( !styles[ style ].observable ) {
 				// Just return style as `property-name: value`.
 				// Note that style property has to be transformed from camelCase to dash
-				// for setting initial value by `setAttributeNS()`.
+				// for setting initial value by `el.style.cssText`.
 				return `${ camelCaseToDash( style ) }:${ styles[ style ] }`;
 			}
 

+ 96 - 53
packages/ckeditor5-ui/tests/template.js

@@ -204,59 +204,6 @@ describe( 'Template', () => {
 			expect( el.attributes.getNamedItem( 'x' ).namespaceURI ).to.equal( 'abc' );
 		} );
 
-		it( 'renders HTMLElement attribute style with dynamic and static properties', () => {
-			const observable = new Model( {
-				width: '100px',
-				color: 'yellow'
-			} );
-
-			const emitter = Object.create( EmitterMixin );
-			const bind = Template.bind( observable, emitter );
-			const el = new Template( {
-				tag: 'p',
-				attributes: {
-					style: {
-						width: bind.to( 'width' ),
-						height: '200px',
-						backgroundColor: bind.to( 'color' )
-					}
-				}
-			} ).render();
-
-			expect( el.style.width ).to.be.equal( '100px' );
-			expect( el.style.height ).to.be.equal( '200px' );
-			expect( el.style.backgroundColor ).to.be.equal( 'yellow' );
-
-			observable.width = '50px';
-			observable.color = 'green';
-
-			expect( el.style.width ).to.be.equal( '50px' );
-			expect( el.style.height ).to.be.equal( '200px' );
-			expect( el.style.backgroundColor ).to.be.equal( 'green' );
-		} );
-
-		it( 'renders HTMLElement attribute style fully binded', () => {
-			const observable = new Model( {
-				style: 'width: 100px'
-			} );
-
-			const emitter = Object.create( EmitterMixin );
-			const bind = Template.bind( observable, emitter );
-			const el = new Template( {
-				tag: 'p',
-				attributes: {
-					style: bind.to( 'style' )
-				}
-			} ).render();
-
-			expect( el.style.width ).to.be.equal( '100px' );
-
-			observable.style = 'width:20px;height:50px';
-
-			expect( el.style.width ).to.be.equal( '20px' );
-			expect( el.style.height ).to.be.equal( '50px' );
-		} );
-
 		it( 'creates HTMLElement children', () => {
 			const el = new Template( {
 				tag: 'p',
@@ -368,6 +315,102 @@ describe( 'Template', () => {
 			observable.foo = 'baz';
 			expect( el.firstChild.textContent ).to.equal( 'baz static' );
 		} );
+
+		describe( '`style` attribute', () => {
+			it( 'renders with dynamic and static properties', () => {
+				const observable = new Model( {
+					width: '100px',
+					color: 'yellow'
+				} );
+
+				const emitter = Object.create( EmitterMixin );
+				const bind = Template.bind( observable, emitter );
+				const el = new Template( {
+					tag: 'p',
+					attributes: {
+						style: {
+							width: bind.to( 'width' ),
+							height: '200px',
+							backgroundColor: bind.to( 'color' )
+						}
+					}
+				} ).render();
+
+				expect( el.style.width ).to.be.equal( '100px' );
+				expect( el.style.height ).to.be.equal( '200px' );
+				expect( el.style.backgroundColor ).to.be.equal( 'yellow' );
+
+				observable.width = '50px';
+				observable.color = 'green';
+
+				expect( el.style.width ).to.be.equal( '50px' );
+				expect( el.style.height ).to.be.equal( '200px' );
+				expect( el.style.backgroundColor ).to.be.equal( 'green' );
+			} );
+
+			it( 'renders fully binded', () => {
+				const observable = new Model( {
+					style: 'width: 100px'
+				} );
+
+				const emitter = Object.create( EmitterMixin );
+				const bind = Template.bind( observable, emitter );
+				const el = new Template( {
+					tag: 'p',
+					attributes: {
+						style: bind.to( 'style' )
+					}
+				} ).render();
+
+				expect( el.style.width ).to.be.equal( '100px' );
+
+				observable.style = 'width:20px;height:50px';
+
+				expect( el.style.width ).to.be.equal( '20px' );
+				expect( el.style.height ).to.be.equal( '50px' );
+			} );
+
+			it( 'throws when wrong format for initial value of fully binded', () => {
+				const observable = new Model( {
+					style: {
+						width: '100px'
+					}
+				} );
+
+				const emitter = Object.create( EmitterMixin );
+				const bind = Template.bind( observable, emitter );
+
+				expect( () => {
+					new Template( {
+						tag: 'p',
+						attributes: {
+							style: bind.to( 'style' )
+						}
+					} ).render();
+				} ).to.throw( CKEditorError, /template-renderAttributeStyle-invalid-format/ );
+			} );
+
+			it( 'throws when wrong format for updated value of fully binded', () => {
+				const observable = new Model( {
+					style: 'width: 100px'
+				} );
+
+				const emitter = Object.create( EmitterMixin );
+				const bind = Template.bind( observable, emitter );
+				new Template( {
+					tag: 'p',
+					attributes: {
+						style: bind.to( 'style' )
+					}
+				} ).render();
+
+				expect( () => {
+					observable.style = {
+						width: '100px'
+					};
+				} ).to.throw( CKEditorError, /template-renderAttributeStyle-invalid-format/ );
+			} );
+		} );
 	} );
 
 	describe( 'apply', () => {