Merge pull request #1718 from ckeditor/t/1717

Fix: Make sure that `schema#getAttributeProperties()` always return an object. Closes #1717.
ckeditor · Apr 3, 2019 · b3f5da3 · b3f5da3
2 parents c48f5a4 + bd3d37d
commit b3f5da3
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 21 deletions.
diff --git a/src/model/schema.js b/src/model/schema.js
@@ -522,7 +522,7 @@ export default class Schema {
 	 * @param {module:engine/model/schema~AttributeProperties} properties A dictionary of properties.
 	 */
 	setAttributeProperties( attributeName, properties ) {
-		this._attributeProperties[ attributeName ] = Object.assign( this.getAttributeProperties( attributeName ) || {}, properties );
+		this._attributeProperties[ attributeName ] = Object.assign( this.getAttributeProperties( attributeName ), properties );
 	}
 
 	/**
@@ -532,7 +532,7 @@ export default class Schema {
 	 * @returns {module:engine/model/schema~AttributeProperties}
 	 */
 	getAttributeProperties( attributeName ) {
-		return this._attributeProperties[ attributeName ];
+		return this._attributeProperties[ attributeName ] || {};
 	}
 
 	/**

diff --git a/tests/model/schema.js b/tests/model/schema.js
@@ -96,7 +96,7 @@ describe( 'Schema', () => {
 		} );
 	} );
 
-	describe( 'setAttributeProperties()', () => {
+	describe( 'attribute properties', () => {
 		beforeEach( () => {
 			schema.register( '$root' );
 			schema.register( 'paragraph', {
@@ -105,33 +105,45 @@ describe( 'Schema', () => {
 			schema.register( '$text', {
 				allowIn: 'paragraph'
 			} );
-			schema.extend( '$text', { allowAttributes: 'testAttribute' } );
+			schema.extend( '$text', { allowAttributes: [ 'testAttribute', 'noPropertiesAttribute' ] } );
 		} );
 
-		it( 'allows registering new properties', () => {
-			schema.setAttributeProperties( 'testAttribute', {
-				foo: 'bar',
-				baz: 'bom'
-			} );
+		describe( 'setAttributeProperties()', () => {
+			it( 'allows registering new properties', () => {
+				schema.setAttributeProperties( 'testAttribute', {
+					foo: 'bar',
+					baz: 'bom'
+				} );
 
-			expect( schema.getAttributeProperties( 'testAttribute' ) ).to.deep.equal( {
-				foo: 'bar',
-				baz: 'bom'
+				expect( schema.getAttributeProperties( 'testAttribute' ) ).to.deep.equal( {
+					foo: 'bar',
+					baz: 'bom'
+				} );
 			} );
-		} );
 
-		it( 'support adding properties in subsequent calls', () => {
-			schema.setAttributeProperties( 'testAttribute', {
-				first: 'foo'
+			it( 'support adding properties in subsequent calls', () => {
+				schema.setAttributeProperties( 'testAttribute', {
+					first: 'foo'
+				} );
+
+				schema.setAttributeProperties( 'testAttribute', {
+					second: 'bar'
+				} );
+
+				expect( schema.getAttributeProperties( 'testAttribute' ) ).to.deep.equal( {
+					first: 'foo',
+					second: 'bar'
+				} );
 			} );
+		} );
 
-			schema.setAttributeProperties( 'testAttribute', {
-				second: 'bar'
+		describe( 'getAttributeProperties()', () => {
+			it( 'it returns a proper value if the attribute has no properties', () => {
+				expect( schema.getAttributeProperties( 'noPropertiesAttribute' ) ).to.deep.equal( {} );
 			} );
 
-			expect( schema.getAttributeProperties( 'testAttribute' ) ).to.deep.equal( {
-				first: 'foo',
-				second: 'bar'
+			it( 'it returns a proper value for unknown attribute', () => {
+				expect( schema.getAttributeProperties( 'unregistered-attribute' ) ).to.deep.equal( {} );
 			} );
 		} );
 	} );