Merge pull request ckeditor#14168 from ckeditor/ck/14025-multiple-men…

…tions-in-the-last-cell-result-in-selection-error Fix (mention): Multiple mentions in the last table cell should not result in a selection error. Closes ckeditor#14025.
choijiho0021 · May 23, 2023 · 39c2d91 · 39c2d91
2 parents 48fa344 + 7fd56ff
commit 39c2d91
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 4 deletions.
diff --git a/packages/ckeditor5-mention/tests/mention-integration.js b/packages/ckeditor5-mention/tests/mention-integration.js
@@ -13,6 +13,7 @@ import TableToolbar from '@ckeditor/ckeditor5-table/src/tabletoolbar';
 import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
 import Link from '@ckeditor/ckeditor5-link/src/link';
 import Delete from '@ckeditor/ckeditor5-typing/src/delete';
+import DomEventData from '@ckeditor/ckeditor5-engine/src/view/observer/domeventdata';
 import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
 
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
@@ -333,4 +334,61 @@ describe( 'Mention feature - integration', () => {
 				} );
 		} );
 	} );
+
+	describe( 'with table', () => {
+		beforeEach( () => {
+			return ClassicTestEditor
+				.create( div, {
+					plugins: [ Paragraph, Table, Mention ],
+					mention: {
+						feeds: [
+							{
+								marker: '@',
+								feed: [ '@Barney' ]
+							}
+						]
+					}
+				} )
+				.then( newEditor => {
+					editor = newEditor;
+				} );
+		} );
+
+		it( 'should not throw on backspace: selection after table containing 2 mentions in the last cell', () => {
+			const viewDocument = editor.editing.view.document;
+
+			// Insert table with 2 mentions in the last cell
+			expect( () => {
+				editor.setData(
+					'<figure class="table"><table><tbody><tr><td>' +
+						'<span class="mention" data-mention="@Barney">@Barney</span> ' +
+						'<span class="mention" data-mention="@Barney">@Barney</span>' +
+					'</td></tr></tbody></table></figure><p>&nbsp;</p>' );
+			} ).not.to.throw();
+
+			// Set selection after the table
+			editor.model.change( writer => {
+				const paragraph = editor.model.document.getRoot().getChild( 1 );
+
+				writer.setSelection( paragraph, 0 );
+			} );
+
+			const deleteEvent = new DomEventData(
+				viewDocument,
+				{ preventDefault: sinon.spy() },
+				{ direction: 'backward', unit: 'codePoint', sequence: 1 }
+			);
+
+			expect( () => {
+				viewDocument.fire( 'delete', deleteEvent );
+			} ).not.to.throw();
+
+			expect( editor.getData() ).to.equal(
+				'<figure class="table"><table><tbody><tr><td>' +
+					'<span class="mention" data-mention="@Barney">@Barney</span> ' +
+					'<span class="mention" data-mention="@Barney">@Barney</span>' +
+				'</td></tr></tbody></table></figure>'
+			);
+		} );
+	} );
 } );
diff --git a/packages/ckeditor5-typing/src/utils/getlasttextline.ts b/packages/ckeditor5-typing/src/utils/getlasttextline.ts
@@ -39,15 +39,15 @@ import type { Model, Range } from '@ckeditor/ckeditor5-engine';
 export default function getLastTextLine( range: Range, model: Model ): LastTextLineData {
 	let start = range.start;
 
-	const text = Array.from( range.getItems() ).reduce( ( rangeText, node ) => {
+	const text = Array.from( range.getWalker( { ignoreElementEnd: false } ) ).reduce( ( rangeText, { item } ) => {
 		// Trim text to a last occurrence of an inline element and update range start.
-		if ( !( node.is( '$text' ) || node.is( '$textProxy' ) ) ) {
-			start = model.createPositionAfter( node );
+		if ( !( item.is( '$text' ) || item.is( '$textProxy' ) ) ) {
+			start = model.createPositionAfter( item );
 
 			return '';
 		}
 
-		return rangeText + node.data;
+		return rangeText + item.data;
 	}, '' );
 
 	return { text, range: model.createRange( start, range.end ) };

diff --git a/packages/ckeditor5-typing/tests/utils/getlasttextline.js b/packages/ckeditor5-typing/tests/utils/getlasttextline.js
@@ -76,6 +76,16 @@ describe( 'utils', () => {
 				[ 0, 0 ],
 				[ 0, 9 ] );
 		} );
+
+		it( 'should return empty string if the range is `on` the element', () => {
+			setModelData( model, '<paragraph>foobarbaz[]</paragraph>' );
+
+			testOutput(
+				model.createRangeOn( root.getChild( 0 ) ),
+				'',
+				[ 1 ],
+				[ 1 ] );
+		} );
 	} );
 
 	function testOutput( range1, expectedText, startPath, endPath ) {