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

Editor crashes after merging paragraph to blockquote in table cell #1265

Closed
Mgsy opened this issue Sep 26, 2018 · 3 comments
Closed

Editor crashes after merging paragraph to blockquote in table cell #1265

Mgsy opened this issue Sep 26, 2018 · 3 comments
Assignees
Labels
type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@Mgsy
Copy link
Member

Mgsy commented Sep 26, 2018

Is this a bug report or feature request? (choose one)

🐞 Bug report

💻 Version of CKEditor

Latest master.

📋 Steps to reproduce

  1. Go to https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/examples/builds/document-editor.html.
  2. Insert 1x2 table.
  3. In the first cell insert a blockquote.
  4. In the second cell type something.
  5. Merge both cells.
  6. Put the caret at the beginning of the paragraph in the merged cell.
  7. Press Backspace.

✅ Expected result

The editor doesn't crash.

❎ Actual result

The editor crashes.

📃 Other details that might be useful

Error

moveoperation.js:171 Uncaught CKEditorError: move-operation-node-into-itself: Trying to move a range of nodes into one of nodes from that range. Read more: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-move-operation-node-into-itself
    at MoveOperation._validate (webpack://Letters/./packages/@ckeditor/ckeditor5-engine/src/model/operation/moveoperation.js?:171:12)
    at Model.on (webpack://Letters/./packages/@ckeditor/ckeditor5-engine/src/model/model.js?:99:14)
    at Model.fire (webpack://Letters/./packages/@ckeditor/ckeditor5-utils/src/emittermixin.js?:203:29)
    at Model.(anonymous function) [as applyOperation] (webpack://Letters/./packages/@ckeditor/ckeditor5-utils/src/observablemixin.js?:257:16)
    at Writer.move (webpack://Letters/./packages/@ckeditor/ckeditor5-engine/src/model/writer.js?:507:14)
    at Writer.insert (webpack://Letters/./packages/@ckeditor/ckeditor5-engine/src/model/writer.js?:193:10)
    at mergeBranches (webpack://Letters/./packages/@ckeditor/ckeditor5-engine/src/model/utils/deletecontent.js?:153:10)
    at mergeBranches (webpack://Letters/./packages/@ckeditor/ckeditor5-engine/src/model/utils/deletecontent.js?:175:2)
    at model.change.writer (webpack://Letters/./packages/@ckeditor/ckeditor5-engine/src/model/utils/deletecontent.js?:88:4)
    at Model.change (webpack://Letters/./packages/@ckeditor/ckeditor5-engine/src/model/model.js?:173:11)

GIF
bug_cke5

@Mgsy Mgsy added the type:bug This issue reports a buggy (incorrect) behavior. label Sep 26, 2018
@Reinmar
Copy link
Member

Reinmar commented Sep 26, 2018

OK, some weird thing is happening here:

  1. image

  2. image

  3. image

It looks like we're not handling this Backspace but leave it to the browser (which is the one creating that weird <span>). That should not happen – we should be handling that Backspace...

@Reinmar Reinmar added this to the iteration 20 milestone Sep 26, 2018
@jodator jodator assigned jodator and unassigned pomek Sep 27, 2018
@jodator
Copy link
Contributor

jodator commented Sep 27, 2018

Taking this as already on it: https://github.com/cksource/cloud-features/issues/2205.

@pomek found out that this case does not happen without table and it isn't associated with table post fixers.

@pomek
Copy link
Member

pomek commented Sep 27, 2018

Fix for the issue: https://github.com/ckeditor/ckeditor5-engine/blob/99def977bebea4ae342df535cd133d8f63df5825/src/model/utils/deletecontent.js#L142-L148

Here we should check whether endParent and startPos.parent aren't the same elements.

if ( endParent == startPos.parent ) {
	return;
}

if ( !endPos.isEqual( startPos ) ) {
	// In this case, before we merge, we need to move `endParent` to the `startPos`:
	// <a><b>x[]</b></a><c><d>{}y</d></c>
	// becomes:
	// <a><b>x</b>[]<d>y</d></a><c>{}</c>

	writer.insert( endParent, startPos );
}

@jodator jodator assigned pomek and unassigned jodator Sep 27, 2018
@Reinmar Reinmar modified the milestones: iteration 20, iteration 21 Oct 2, 2018
Reinmar added a commit to ckeditor/ckeditor5-engine that referenced this issue Oct 26, 2018
Fix: `model#deleteContent()` will proper merge elements inside limit element. Closes ckeditor/ckeditor5#1265.
Reinmar added a commit to ckeditor/ckeditor5-table that referenced this issue Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

4 participants