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

Crash when undoing typing (model-nodelist-offset-out-of-bounds) #7903

Closed
oleq opened this issue Aug 21, 2020 · 6 comments
Closed

Crash when undoing typing (model-nodelist-offset-out-of-bounds) #7903

oleq opened this issue Aug 21, 2020 · 6 comments
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:engine resolution:expired This issue was closed due to lack of feedback. squad:core Issue to be handled by the Core team. status:stale type:bug This issue reports a buggy (incorrect) behavior. type:regression This issue reports a bug that was not present in the previous releases.

Comments

@oleq
Copy link
Member

oleq commented Aug 21, 2020

📝 Provide detailed reproduction steps (if any)

👀 The issue should get fixed by #7462 👀

  1. Go to http://fake.ckeditor.com/ck/5/ckeditor5/build/docs/ckeditor5/latest/examples/builds/document-editor.html.
  2. Select the table.
  3. Type "foo"
  4. Ctrl+K
  5. Ctrl+Z

 See the screencast:

Uncaught CKEditorError: model-nodelist-offset-out-of-bounds: Given offset cannot be found in the node list. Read more: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-model-nodelist-offset-out-of-bounds
 {"offset":3,"nodeList":[]}
    at NodeList.offsetToIndex (webpack:///./packages/ckeditor5-engine/src/model/nodelist.js?:165:10)
    at Element.offsetToIndex (webpack:///./packages/ckeditor5-engine/src/model/element.js?:193:25)
    at getTextNodeAtPosition (webpack:///./packages/ckeditor5-engine/src/model/position.js?:1113:55)
    at Position.get textNode [as textNode] (webpack:///./packages/ckeditor5-engine/src/model/position.js?:227:10)
    at validateTextNodePosition (webpack:///./packages/ckeditor5-engine/src/model/document.js?:493:33)
    at Document._validateSelectionRange (webpack:///./packages/ckeditor5-engine/src/model/document.js?:402:10)
    at LiveSelection.eval (webpack:///./packages/ckeditor5-engine/src/model/documentselection.js?:659:26)
    at LiveSelection.fire (webpack:///./packages/ckeditor5-utils/src/emittermixin.js?:218:30)
    at LiveSelection._setRanges (webpack:///./packages/ckeditor5-engine/src/model/selection.js?:488:8)
    at LiveSelection.setTo (webpack:///./packages/ckeditor5-engine/src/model/selection.js?:385:9)
CKEditorError @ ckeditorerror.js:70
offsetToIndex @ nodelist.js:165
offsetToIndex @ element.js:193
getTextNodeAtPosition @ position.js:1113
get textNode @ position.js:227
validateTextNodePosition @ document.js:493
_validateSelectionRange @ document.js:402
eval @ documentselection.js:659
fire @ emittermixin.js:218
_setRanges @ selection.js:488
setTo @ selection.js:385
setTo @ documentselection.js:747
_setTo @ documentselection.js:433
setSelection @ writer.js:1209
eval @ inputcommand.js:106
_runPendingChanges @ model.js:851
enqueueChange @ model.js:257
execute @ inputcommand.js:93
eval @ observablemixin.js:259
fire @ emittermixin.js:218
<computed> @ observablemixin.js:263
execute @ commandcollection.js:72
execute @ editor.js:306
_handleContainerChildrenMutations @ injecttypingmutationshandling.js:183
handle @ injecttypingmutationshandling.js:69
eval @ injecttypingmutationshandling.js:27
fire @ emittermixin.js:218
_onMutations @ mutationobserver.js:255

✔️ Expected result

What is the expected result of the above steps?

❌ Actual result

What is the actual result of the above steps?

📃 Other details

Regression since v20.0.0.

  • Browser: Chrome
  • OS: Mac
  • CKEditor version: master (57d3f02)

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

@oleq oleq added type:bug This issue reports a buggy (incorrect) behavior. package:engine labels Aug 21, 2020
@oleq oleq changed the title Crash when opening the link balloon (model-nodelist-offset-out-of-bounds) Crash when undoing typing (model-nodelist-offset-out-of-bounds) Aug 21, 2020
@FilipTokarski
Copy link
Member

FilipTokarski commented Aug 21, 2020

I see that it's also reproducible on live docs, happens since version 20.0.0.
You can also boil down the steps to just:

  • type something in empty paragraph
  • ctrl+K
  • ctrl+Z

@oleq oleq added this to the iteration 35 milestone Aug 21, 2020
@Reinmar Reinmar added the release:blocker This issue blocks the upcoming release (is critical). label Aug 21, 2020
@Reinmar Reinmar modified the milestones: iteration 35, iteration 36 Aug 24, 2020
@mlewand mlewand added type:regression This issue reports a bug that was not present in the previous releases. domain:ui/ux This issue reports a problem related to UI or UX. squad:core Issue to be handled by the Core team. and removed release:blocker This issue blocks the upcoming release (is critical). labels Aug 24, 2020
@Reinmar
Copy link
Member

Reinmar commented Aug 26, 2020

Idea: If we won't be able to find the root of the problem (why the mutation happens when we press ctrl+z which is then preventDefault()ed), we agreed that it'd be ok to remove the fake caret (the one for collapsed selection) completely. The UX without the caret wasn't bad. We introduced it while we were working on a fake selection for non-collapsed selections, which was a much bigger problem itself.

@mlewand
Copy link
Contributor

mlewand commented Sep 9, 2020

Similar issue but with a ranged selection: #8052

@mlewand
Copy link
Contributor

mlewand commented Sep 10, 2020

The reason for this uncontrolled behavior is a Chrome feature (that's why FF doesn't suffer from it) where if you undo input that has no more undo steps it will move your focus to the previously focused component and undo it for you.

We could go for easy win by blocking ctrl+z/y in this case - but that would just hide the problem.

Instead I'm seeking for the source of the problem. Also I was able to create a unit test using document.execCommand() API.


After identifying that the problem comes from our mutation handler I checked whether a fix for #7462 doesn't solve this and indeed it fixes the issue (branch i/7462-beforeinput-poc, hash f639b8b). I have pushed an unit test in the i/7903 branch for easy verification that the issue was fixed.

@Reinmar Reinmar modified the milestones: iteration 36, iteration 37 Sep 28, 2020
@Reinmar Reinmar modified the milestones: iteration 37, iteration 38 Oct 26, 2020
@AnnaTomanek AnnaTomanek modified the milestones: iteration 38, backlog Dec 4, 2020
@pomek pomek removed this from the backlog milestone Feb 21, 2022
@CKEditorBot
Copy link
Collaborator

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

@CKEditorBot
Copy link
Collaborator

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

@CKEditorBot CKEditorBot added the resolution:expired This issue was closed due to lack of feedback. label Nov 12, 2023
@CKEditorBot CKEditorBot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:engine resolution:expired This issue was closed due to lack of feedback. squad:core Issue to be handled by the Core team. status:stale type:bug This issue reports a buggy (incorrect) behavior. type:regression This issue reports a bug that was not present in the previous releases.
Projects
None yet
Development

No branches or pull requests

7 participants