getSelectedBlocks() should ignore blocks where no content is selected

packages/ckeditor5-engine/src/model/selection.ts

@@ -637,13 +637,23 @@ export default class Selection extends EmitterMixin( TypeCheckable ) {
 	 * </block>
 	 * ```
 	 *
-	 * **Special case**: If a selection ends at the beginning of a block, that block is not returned as from user perspective
-	 * this block wasn't selected. See [#984](https://github.com/ckeditor/ckeditor5-engine/issues/984) for more details.
+	 * **Special case**: Selection ignores first and/or last blocks if nothing (from user perspective) is selected in them.
 	 *
 	 * ```xml
+	 * // Selection ends and the beginning of the last block
 	 * <paragraph>[a</paragraph>
 	 * <paragraph>b</paragraph>
-	 * <paragraph>]c</paragraph> // this block will not be returned
+	 * <paragraph>]c</paragraph> // This block will not be returned
+	 *
+	 * // Selection begins at the end of the first block
+	 * <paragraph>a[</paragraph> // This block will not be returned
+	 * <paragraph>b</paragraph>
+	 * <paragraph>c]</paragraph>
+	 *
+	 * // Selection begings at the end of the first block and ends at the beginning of the last block
+	 * <paragraph>a[</paragraph> // This block will not be returned
+	 * <paragraph>b</paragraph>
+	 * <paragraph>]c</paragraph> // This block will not be returned
 	 * ```
 	 */
 	public* getSelectedBlocks(): IterableIterator<Element> {
@@ -653,7 +663,7 @@ export default class Selection extends EmitterMixin( TypeCheckable ) {
 			// Get start block of range in case of a collapsed range.
 			const startBlock = getParentBlock( range.start, visited );
 
-			if ( startBlock && isTopBlockInRange( startBlock, range ) ) {
+			if ( isStartBlockSelected( startBlock, range ) ) {
 				yield startBlock as any;
 			}
 
@@ -667,8 +677,7 @@ export default class Selection extends EmitterMixin( TypeCheckable ) {
 
 			const endBlock = getParentBlock( range.end, visited );
 
-			// #984. Don't return the end block if the range ends right at its beginning.
-			if ( endBlock && !range.end.isTouching( Position._createAt( endBlock, 0 ) ) && isTopBlockInRange( endBlock, range ) ) {
+			if ( isEndBlockSelected( endBlock, range ) ) {
 				yield endBlock as any;
 			}
 		}
@@ -876,6 +885,70 @@ function isTopBlockInRange( block: Node, range: Range ) {
 	return !isParentInRange;
 }
 
+/**
+ * If a selection starts at the end of a block, that block is not returned as from user perspective this block wasn't selected.
+ * See [#11585](https://github.com/ckeditor/ckeditor5/issues/11585) for more details.
+ *
+ * ```xml
+ * <paragraph>a[</paragraph> // this block will not be returned
+ * <paragraph>b</paragraph>
+ * <paragraph>c]</paragraph>
+ * ```
+ *
+ * Collapsed selection is not affected by it:
+ *
+ * ```xml
+ * <paragraph>a[]</paragraph> // this block will be returned
+ * ```
+ */
+function isStartBlockSelected( startBlock: Element | undefined, range: Range ): boolean {
+	if ( !startBlock ) {
+		return false;
+	}
+
+	if ( range.isCollapsed || startBlock.isEmpty ) {
+		return true;
+	}
+
+	if ( range.start.isTouching( Position._createAt( startBlock, startBlock.maxOffset ) ) ) {
+		return false;
+	}
+
+	return isTopBlockInRange( startBlock, range );
+}
+
+/**
+ * If a selection ends at the beginning of a block, that block is not returned as from user perspective this block wasn't selected.
+ * See [#984](https://github.com/ckeditor/ckeditor5-engine/issues/984) for more details.
+ *
+ * ```xml
+ * <paragraph>[a</paragraph>
+ * <paragraph>b</paragraph>
+ * <paragraph>]c</paragraph> // this block will not be returned
+ * ```
+ *
+ * Collapsed selection is not affected by it:
+ *
+ * ```xml
+ * <paragraph>[]a</paragraph> // this block will be returned
+ * ```
+ */
+function isEndBlockSelected( endBlock: Element | undefined, range: Range ): boolean {
+	if ( !endBlock ) {
+		return false;
+	}
+
+	if ( range.isCollapsed || endBlock.isEmpty ) {
+		return true;
+	}
+
+	if ( range.end.isTouching( Position._createAt( endBlock, 0 ) ) ) {
+		return false;
+	}
+
+	return isTopBlockInRange( endBlock, range );
+}
+
 /**
  * Returns first ancestor block of a node.
  */

packages/ckeditor5-engine/tests/model/selection.js

@@ -1001,10 +1001,10 @@ describe( 'Selection', () => {
 			expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'h#b', 'p#c' ] );
 		} );
 
-		it( 'returns two blocks for a non collapsed selection (starts at block end)', () => {
+		it( 'returns one block for a non collapsed selection (starts at block end)', () => {
 			setData( model, '<p>a</p><h>b[</h><p>c]</p><p>d</p>' );
 
-			expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'h#b', 'p#c' ] );
+			expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'p#c' ] );
 		} );
 
 		it( 'returns proper block for a multi-range selection', () => {
@@ -1139,10 +1139,10 @@ describe( 'Selection', () => {
 				expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'p#a', 'p#b' ] );
 			} );
 
-			it( 'returns only the first block for a non collapsed selection if only end of selection is touching a block', () => {
+			it( 'returns no blocks if selection spanning two blocks has no content', () => {
 				setData( model, '<p>a</p><h>b[</h><p>]c</p><p>d</p>' );
 
-				expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'h#b' ] );
+				expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [] );
 			} );
 
 			it( 'does not return the last block if none of its content is selected (nested case)', () => {
@@ -1182,6 +1182,44 @@ describe( 'Selection', () => {
 				}
 			);
 		} );
+
+		describe( '#11585', () => {
+			it( 'does not return the first block if none of its contents is selected', () => {
+				setData( model, '<p>a[</p><p>b</p><p>c]</p>' );
+
+				expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'p#b', 'p#c' ] );
+			} );
+
+			it( 'returns the first block if at least one of its child nodes is selected', () => {
+				setData( model, '<p>a[<imageBlock></imageBlock></p><p>b</p><p>c]</p>' );
+
+				expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'p#a', 'p#b', 'p#c' ] );
+			} );
+
+			it( 'returns the block if it has a collapsed selection at the beginning', () => {
+				setData( model, '<p>[]a</p><p>b</p>' );
+
+				expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'p#a' ] );
+			} );
+
+			it( 'returns the block if it has a collapsed selection at the end', () => {
+				setData( model, '<p>a[]</p><p>b</p>' );
+
+				expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'p#a' ] );
+			} );
+
+			it( 'does not return first and last blocks if no content is selected', () => {
+				setData( model, '<p>a[</p><p>]b</p>' );
+
+				expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [] );
+			} );
+
+			it( 'returns the first and last blocks if no content is selected but both blocks are empty', () => {
+				setData( model, '<p>[</p><p>]</p>' );
+
+				expect( stringifyBlocks( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'p', 'p' ] );
+			} );
+		} );
 	} );
 
 	describe( 'attributes interface', () => {

packages/ckeditor5-heading/tests/headingcommand.js

@@ -225,12 +225,12 @@ describe( 'HeadingCommand', () => {
 			}
 
 			it( 'converts all elements where selection is applied', () => {
-				setData( model, '<heading1>foo[</heading1><heading2>bar</heading2><heading3>baz]</heading3>' );
+				setData( model, '<heading1>fo[o</heading1><heading2>bar</heading2><heading3>baz]</heading3>' );
 
 				command.execute( { value: 'heading3' } );
 
 				expect( getData( model ) ).to.equal(
-					'<heading3>foo[</heading3><heading3>bar</heading3><heading3>baz]</heading3>'
+					'<heading3>fo[o</heading3><heading3>bar</heading3><heading3>baz]</heading3>'
 				);
 			} );
 

packages/ckeditor5-list/tests/documentlistproperties/documentliststartcommand.js

@@ -142,9 +142,9 @@ describe( 'DocumentListStartCommand', () => {
 
 		it( 'should return the value of `listStart` attribute from a list where the selection starts (selection over nested list)', () => {
 			setData( model, modelList( `
-				# 1. {start:2}
-				  # 1.1.[ {start:3}
-				# 2.]
+				# 1. First {start:2}
+				  # 1.1. [Second {start:3}
+				# 2. Third]
 			` ) );
 
 			expect( listStartCommand.value ).to.equal( 3 );

packages/ckeditor5-list/tests/documentlistproperties/documentliststylecommand.js

@@ -127,9 +127,9 @@ describe( 'DocumentListStyleCommand', () => {
 
 		it( 'should return the value of `listStyle` attribute from a list where the selection starts (selection over nested list)', () => {
 			setData( model, modelList( `
-				* 1. {style:square}
-				  * 1.1.[ {style:disc}
-				* 2.]
+				* 1. First {style:square}
+				  * 1.1. [Second {style:disc}
+				* 2. Third]
 			` ) );
 
 			expect( listStyleCommand.value ).to.equal( 'disc' );

packages/ckeditor5-paragraph/tests/paragraphcommand.js

@@ -219,11 +219,11 @@ describe( 'ParagraphCommand', () => {
 			it( 'converts all elements where selection is applied', () => {
 				schema.register( 'heading2', { inheritAllFrom: '$block' } );
 
-				setData( model, '<heading1>foo[</heading1><heading2>bar</heading2><heading2>baz]</heading2>' );
+				setData( model, '<heading1>fo[o</heading1><heading2>bar</heading2><heading2>baz]</heading2>' );
 
 				command.execute();
 				expect( getData( model ) ).to.equal(
-					'<paragraph>foo[</paragraph><paragraph>bar</paragraph><paragraph>baz]</paragraph>'
+					'<paragraph>fo[o</paragraph><paragraph>bar</paragraph><paragraph>baz]</paragraph>'
 				);
 			} );