Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple mentions in the last table cell results in a selection error #14025

Closed
MichaelHeydon opened this issue May 4, 2023 · 1 comment · Fixed by #14168
Closed

Multiple mentions in the last table cell results in a selection error #14025

MichaelHeydon opened this issue May 4, 2023 · 1 comment · Fixed by #14168
Assignees
Labels
package:mention package:table squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@MichaelHeydon
Copy link

📝 Provide detailed reproduction steps (if any)

  1. Visit https://ckeditor.com/docs/ckeditor5/latest/features/mentions.html
  2. In the first Demo, remove Hello @ted.
  3. Create a table with a single cell
  4. Add two mentions
  5. Add a new line so the editor looks like:
    screenshot-ckeditor com-2023 05 04-10_23_48
  6. Attempt to select the table by pressing backspace.
  7. The following error is logged in the console:
    mention-error
snippet.js:4 Uncaught CKEditorError: model-nodelist-offset-out-of-bounds {"offset":6,"nodeList":[{"name":"table","children":[{"name":"tableRow","children":[{"name":"tableCell","children":[{"name":"paragraph","children":[{"attributes":{"mention":{"uid":"e4dc38be825796bd7007c8c3f16e9ba63","_text":"@Ted","id":"@Ted","text":"@Ted"}},"data":"@Ted"},{"data":"  "},{"attributes":{"mention":{"uid":"e6f4dc4a2dde9a5bfed0aa66b11e6f9dc","_text":"@Barney","id":"@Barney","text":"@Barney"}},"data":"@Barney"},{"data":" "}]}]}]}]}]}
Read more: https://ckeditor.com/docs/ckeditor5/latest/support/error-codes.html#error-model-nodelist-offset-out-of-bounds
    at ol.offsetToIndex (snippet.js:4:153446)
    at Eh.offsetToIndex (snippet.js:4:156105)
    at ul (snippet.js:4:166656)
    at get nodeAfter [as nodeAfter] (snippet.js:4:161159)
    at snippet.js:4:441773
    at fm.<anonymous> (snippet.js:4:441834)
    at fm.fire (snippet.js:4:5541)
    at fm._evaluateTextBeforeSelection (snippet.js:4:432441)
    at Ph.<anonymous> (snippet.js:4:432049)
    at Ph.fire (snippet.js:4:5541)

✔️ Expected result

I can continue to select / manipulate the table.

❌ Actual result

The above error occurs. In this scenario the table can still be selected with the mouse, however in our setup that led to this bug report, the editor becomes entirely unusable.

The issue does not occur when:

  • Having one mention in the table cell
  • Having a column to the right
  • Having a row below

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@MichaelHeydon MichaelHeydon added the type:bug This issue reports a buggy (incorrect) behavior. label May 4, 2023
@Witoso Witoso added squad:core Issue to be handled by the Core team. package:table package:mention labels May 5, 2023
@niegowski
Copy link
Contributor

This most probably should be fixed by replacing this:

const text = Array.from( range.getItems() ).reduce( ( rangeText, node ) => {
// 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 );
return '';
}
return rangeText + node.data;
}, '' );

with:

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 ( !( item.is( '$text' ) || item.is( '$textProxy' ) ) ) {
		start = model.createPositionAfter( item );

		return '';
	}

	return rangeText + item.data;
}, '' );

@CKEditorBot CKEditorBot added the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label May 8, 2023
@mmotyczynska mmotyczynska self-assigned this May 15, 2023
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels May 15, 2023
niegowski added a commit that referenced this issue May 23, 2023
…-the-last-cell-result-in-selection-error

Fix (mention): Multiple mentions in the last table cell should not result in a selection error. Closes #14025.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label May 23, 2023
@CKEditorBot CKEditorBot added this to the iteration 64 milestone May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:mention package:table squad:core Issue to be handled by the Core team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants