Merge pull request #72 from ckeditor/t/56

Fix: The value of the `AttributeCommand` is taken from the first allowed node. Closes #56.

Author: Piotr Jasiun

src/attributecommand.js

@@ -41,7 +41,7 @@ export default class AttributeCommand extends Command {
 		 * Flag indicating whether the command is active. The command is active when the
 		 * {@link module:engine/model/selection~Selection#hasAttribute selection has the attribute} which means that:
 		 *
-		 * * If the selection is not empty – That it starts in a text (or another node) which has the attribute set.
+		 * * If the selection is not empty – That the attribute is set on the first node in the selection that allows this attribute.
 		 * * If the selection is empty – That the selection has the attribute itself (which means that newly typed
 		 * text will have this attribute, too).
 		 *
@@ -58,7 +58,7 @@ export default class AttributeCommand extends Command {
 		const model = this.editor.model;
 		const doc = model.document;
 
-		this.value = doc.selection.hasAttribute( this.attributeKey );
+		this.value = this._getValueFromFirstAllowedNode();
 		this.isEnabled = model.schema.checkAttributeInSelection( doc.selection, this.attributeKey );
 	}
 
@@ -108,4 +108,31 @@ export default class AttributeCommand extends Command {
 			}
 		} );
 	}
+
+	/**
+	 * Checks the attribute value of the first node in the selection that allows the attribute.
+	 * For the collapsed selection returns the selection attribute.
+	 *
+	 * @private
+	 * @returns {Boolean} The attribute value.
+	 */
+	_getValueFromFirstAllowedNode() {
+		const model = this.editor.model;
+		const schema = model.schema;
+		const selection = model.document.selection;
+
+		if ( selection.isCollapsed ) {
+			return selection.hasAttribute( this.attributeKey );
+		}
+
+		for ( const range of selection.getRanges() ) {
+			for ( const item of range.getItems() ) {
+				if ( schema.checkAttribute( item, this.attributeKey ) ) {
+					return item.hasAttribute( this.attributeKey );
+				}
+			}
+		}
+
+		return false;
+	}
 }

tests/attributecommand.js

@@ -49,15 +49,15 @@ describe( 'AttributeCommand', () => {
 	} );
 
 	describe( 'value', () => {
-		it( 'is true when selection has the attribute', () => {
+		it( 'is true when collapsed selection has the attribute', () => {
 			model.change( writer => {
 				writer.setSelectionAttribute( attrKey, true );
 			} );
 
 			expect( command.value ).to.be.true;
 		} );
 
-		it( 'is false when selection does not have the attribute', () => {
+		it( 'is false when collapsed selection does not have the attribute', () => {
 			model.change( writer => {
 				writer.setSelectionAttribute( attrKey, true );
 			} );
@@ -69,8 +69,31 @@ describe( 'AttributeCommand', () => {
 			expect( command.value ).to.be.false;
 		} );
 
-		// See https://github.com/ckeditor/ckeditor5-core/issues/73#issuecomment-311572827.
-		it( 'is false when selection contains object with nested editable', () => {
+		it( 'is true when the first item that allows attribute has the attribute set #1', () => {
+			setData( model, '<p><$text bold="true">fo[o</$text></p><h1>b]ar</h1>' );
+
+			expect( command.value ).to.be.true;
+		} );
+
+		it( 'is true when the first item that allows attribute has the attribute set #2', () => {
+			setData( model, '<h1>fo[o</h1><p><$text bold="true">f</$text>o]o</p>' );
+
+			expect( command.value ).to.be.true;
+		} );
+
+		it( 'is false when the first item that allows attribute does not have the attribute set #1', () => {
+			setData( model, '<p>b[a<$text bold="true">r</$text></p><h1>fo]o</h1>' );
+
+			expect( command.value ).to.be.false;
+		} );
+
+		it( 'is false when the first item that allows attribute does not have the attribute set #2', () => {
+			setData( model, '<h1>fo[o</h1><p>b<$text bold="true">r</$text>r]</p>' );
+
+			expect( command.value ).to.be.false;
+		} );
+
+		it( 'is true when the first item that allows attribute has the attribute set - object with nested editable', () => {
 			model.schema.register( 'caption', {
 				allowContentOf: '$block',
 				allowIn: 'img',
@@ -85,7 +108,7 @@ describe( 'AttributeCommand', () => {
 
 			expect( command.value ).to.be.false;
 			command.execute();
-			expect( command.value ).to.be.false;
+			expect( command.value ).to.be.true;
 
 			expect( getData( model ) ).to.equal(
 				'<p>[<img><caption><$text bold="true">Some caption inside the image.</$text></caption></img>]</p>'