search

ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.56k stars 3.7k forks source link

[Umbrella] Undo/redo issues #2723

Closed Reinmar closed 1 year ago

Reinmar commented 7 years ago

Since we know that there a lot of issues with undo found after the recent refactoring let's report them in one place.

Reinmar commented 7 years ago

Type: Redo, selection restoring

Selection is restored incorrectly:

jul-06-2017 18-11-20

Reinmar commented 7 years ago

FIXED

Rename leaks to a nearby element.

jul-06-2017 18-14-28

Reinmar commented 7 years ago

FIXED

Additional list empty block appears (wrap, split): jul-06-2017 18-15-54

Reinmar commented 7 years ago

FIXED

Type: Undo, performance

Undo becomes noticeably slow when undoing typing :((((

It seems that typing compression will be a must-have in a short time.

jul-06-2017 18-19-31

Edit by LukaszGudel: It looks much better now, undoing and redoing typing is almost instant. IMO fixed.

Reinmar commented 7 years ago

FIXED

jul-06-2017 18-23-14

basic-transformations.js:67 Uncaught TypeError: Cannot read property 'getNode' of undefined
    at addTransformationCase (basic-transformations.js:67)
    at Object.transform (transform.js:59)
    at Object.transformDeltaSets (transform.js:268)
    at Document.transformDeltas (document.js:359)
    at RedoCommand._undo (basecommand.js:140)
    at Array.editor.document.enqueueChanges (redocommand.js:42)
    at Document.enqueueChanges (document.js:241)
    at RedoCommand.execute (redocommand.js:36)
    at RedoCommand.on (observablemixin.js:327)
    at RedoCommand.fire (emittermixin.js:281)
scofalik commented 7 years ago

Undo becomes noticeably slow when undoing typing :((((

Yes... This was another gain that we got by removing deltas from History. There was much less transformations. I am sure we will spend time on this subject because it cannot work like this. I think there are places where optimisation could be applied, but this needs to be measured and researched.

Later, we can think about some tricks to maybe somehow skip some transformations or something but this has to be:

  1. Done when 99% of cases work (meaning that we solved all issues we know about).
  2. Done very carefully so we won't fall again into some kind of trap like with History.
Reinmar commented 7 years ago

FIXED

jul-07-2017 10-16-09

position.js:158 Uncaught TypeError: this.parent.offsetToIndex is not a function
    at Position.get index [as index] (position.js:158)
    at Position.get textNode [as textNode] (position.js:169)
    at Position.get nodeAfter [as nodeAfter] (position.js:181)
    at _fixItemsIndent (converters.js:629)
    at Document.<anonymous> (converters.js:576)
    at Document.fire (emittermixin.js:281)
    at Document.applyOperation (document.js:173)
    at UndoCommand._undo (basecommand.js:153)
    at Array.editor.document.enqueueChanges (undocommand.js:40)
    at Document.enqueueChanges (document.js:241)

EDIT: This TC is really fragile. You need to be very precise regarding actions.

Reinmar commented 7 years ago

As for performance issues. I did a quick check.

Here's a general overview of a single undo step:

image

20 transformDeltas() calls. Each 10ms. But the further you go back in the undo history, the longer the transformDeltas() calls.

Now, it's very interesting to see what are the widest leafs of this flame chart:

image

Yep, Position() constructors. If you'll scroll more you'll see that there also other constructors there (and they are similarly long), but the problem is that Position repeats hundreds of times.

So, it's very likely that one of the most important issues https://github.com/ckeditor/ckeditor5-engine/issues/897.

Edit: I did a small check and removed cloning from Position.createFromPosition() but it's too simplistic (because there are two position types). We'd need more time to check what we can gain by avoiding cloning all the time.

Reinmar commented 7 years ago

FIXED

jul-10-2017 21-50-55

ckeditorerror.js:38 Uncaught CKEditorError: model-history-wrong-version: Given base version points to the middle of a delta.
    at History._getIndex (http://localhost:8125/ckeditor5-presets/tests/manual/article.js:41285:10)
    at History.getDeltas (http://localhost:8125/ckeditor5-presets/tests/manual/article.js:41187:24)
    at getDeltas.next (<anonymous>)
    at Function.from (native)
    at UndoCommand._undo (http://localhost:8125/ckeditor5-presets/tests/manual/article.js:29872:32)
    at Array.editor.document.enqueueChanges (http://localhost:8125/ckeditor5-presets/tests/manual/article.js:47229:30)
    at Document.enqueueChanges (http://localhost:8125/ckeditor5-presets/tests/manual/article.js:40085:30)
    at UndoCommand.execute (http://localhost:8125/ckeditor5-presets/tests/manual/article.js:47228:24)
    at UndoCommand.on (http://localhost:8125/ckeditor5-presets/tests/manual/article.js:5973:32)
    at UndoCommand.fire (http://localhost:8125/ckeditor5-presets/tests/manual/article.js:4267:29)

The above error was reproduced on branch with a fix for https://github.com/ckeditor/ckeditor5-engine/issues/1001. On the current master first undo works, but the second gives:

basic-transformations.js:67 Uncaught TypeError: Cannot read property 'getNode' of undefined
    at addTransformationCase (basic-transformations.js:67)
    at Object.transform (transform.js:59)
    at Object.transformDeltaSets (transform.js:261)
    at Document.transformDeltas (document.js:359)
    at UndoCommand._undo (basecommand.js:140)
    at Array.editor.document.enqueueChanges (undocommand.js:40)
    at Document.enqueueChanges (document.js:241)
    at UndoCommand.execute (undocommand.js:39)
    at UndoCommand.on (observablemixin.js:327)
    at UndoCommand.fire (emittermixin.js:281)
Reinmar commented 7 years ago

FIXED

Type: Undo, incorrect data

This is not a regression but it's a bug worth checking anyway.

jul-13-2017 18-04-06

Edit by LukaszGudel: Undo block quote is now correctly creating 2 nested list items. Fixed.

scofalik commented 7 years ago

The above might be one of "cannot fix with how converters work now" bug. There are actually 6 tests in integration of undo with lists that are intentionally skipped because they are failing. These are the cases that cannot be (at least easily) solved with current state of how conversion works. We skipped them to deliver Nested Lists feature faster.

If we change conversion to happen on changesDone those cases will be probably easier to solve and then we can get back to them.

Mgsy commented 7 years ago

FIXED

Steps to reproduce

  1. Load the clipboard plugin to the markers test.
  2. Go to http://localhost:8125/ckeditor5-engine/tests/manual/markers.html
  3. Create 2 lines of text.
  4. Select them and press Ctrl + C.
  5. Put the caret inside the marker.
  6. Press Ctrl + V.
  7. Press Ctrl + Z.

Current result

GIF

bug_cke5

Error

position.js:221 Uncaught TypeError: Cannot read property 'root' of undefined
    at Position.compareWith (position.js:221)
    at Position.isEqual (position.js:411)
    at addTransformationCase (basic-transformations.js:396)
    at Object.transform (transform.js:59)
    at Object.transformDeltaSets (transform.js:261)
    at Document.transformDeltas (document.js:359)
    at UndoCommand._undo (basecommand.js:140)
    at Array.editor.document.enqueueChanges (undocommand.js:40)
    at Document.enqueueChanges (document.js:241)
    at UndoCommand.execute (undocommand.js:39)

Other information

OS: Windows 10, MacOS X Browser: Chrome, Safari, Edge, Firefox, Opera Release: 0.10.0

Reinmar commented 7 years ago

FIXED

  1. Copy all.
  2. Paste at the end of the content.
  3. Undo.

aug-09-2017 10-30-51

position.js:221 Uncaught TypeError: Cannot read property 'root' of undefined
    at Position.compareWith (position.js:221)
    at Position.isEqual (position.js:411)
    at addTransformationCase (basic-transformations.js:402)
    at Object.transform (transform.js:59)
    at Object.transformDeltaSets (transform.js:261)
    at Document.transformDeltas (document.js:359)
    at UndoCommand._undo (basecommand.js:140)
    at Array.editor.document.enqueueChanges (undocommand.js:40)
    at Document.enqueueChanges (document.js:241)
    at UndoCommand.execute (undocommand.js:39)
Mgsy commented 7 years ago

FIXED

Steps to reproduce

  1. Put the caret at the beginning of the paragraph.
  2. Press Enter.
  3. Press Ctrl + Z.

Current result

The caret was moved to the end of the paragraph.

GIF

bug_cke5

scofalik commented 7 years ago

Selection restoring doesn't work correctly, we know about it and will change it, hopefully soon.

scofalik commented 7 years ago

After I fixed this issue: https://github.com/ckeditor/ckeditor5-undo/issues/65#issuecomment-321190168 I noticed that undo+redo creates different content than it was at the beginning. It's worth to check why it happens. It fails even for fairly non-complicated copy paste. It has to be checked whether there is a bug in: copy/paste, OT or converters and whether this happens only for content with lists or also in other cases.

aug-10-2017 10-12-09

EDIT: Looks like it happens for copy paste whenever pasting is not into an empty element.

Mgsy commented 7 years ago

FIXED | Type: Redo, incorrect data

Block quote is removed after undo/redo operations.

Steps to reproduce

  1. Create new paragraph and new block quote.
  2. Put the caret at the end of the paragraph.
  3. Press Delete.
  4. Press Ctrl + Z.
  5. Ctrl + Y.

GIF

bug_cke5

Mgsy commented 7 years ago

FIXED | Type: Undo, incorrect data

Steps to reproduce

  1. Create two block elements, i.e. paragraphs.
  2. Put the caret at the end of the first paragraph.
  3. Press Enter.
  4. Press Delete.
  5. Press Ctrl + Z twice.

Current result

The second paragraph has been removed.

GIF

bug_cke5

Mgsy commented 7 years ago

FIXED

Steps to reproduce

  1. Create the empty paragraph.
  2. Press Ctrl + B.
  3. Type something.
  4. Put the caret at the middle of the word.
  5. Press Enter.
  6. Press Undo 4 times.

Current result

There is the error in the console.

Error

basic-transformations.js:270 Uncaught TypeError: Cannot read property 'root' of null
    at addTransformationCase (basic-transformations.js:270)
    at Object.transform (transform.js:59)
    at Object.transformDeltaSets (transform.js:261)
    at Document.transformDeltas (document.js:359)
    at UndoCommand._undo (basecommand.js:140)
    at Array.editor.document.enqueueChanges (undocommand.js:40)
    at Document.enqueueChanges (document.js:241)
    at UndoCommand.execute (undocommand.js:39)
    at UndoCommand.on (observablemixin.js:327)
    at UndoCommand.fire (emittermixin.js:281)

GIF

bug_cke5

Mgsy commented 7 years ago

FIXED | Type: Undo, incorrect data

Steps to reproduce

  1. Put the caret at the middle of some word, i.e. paragraph.
  2. Press Enter.
  3. Make the following selection: 
    parag{
raph}
  4. Press Delete.
  5. Press Ctrl + Z twice.

Current result

Undo doesn't merge the word.

GIF

bug_cke5

Mgsy commented 7 years ago

FIXED | Type: Redo, selection restoring

Steps to reproduce

  1. Create the paragraph below the block quote.
  2. Type something.
  3. Put the caret at the beginning of the paragraph.
  4. Press Backspace.
  5. Press Ctrl + Z.
  6. Press Ctrl + Y.

Current result

Block quote was applied to the paragraph instead of moving paragraph to the end of the block quote.

GIF

bug_cke5

Reinmar commented 7 years ago

FIXED | Type: Undo, incorrect data, crash

See https://github.com/ckeditor/ckeditor5-engine/issues/1152

Mgsy commented 7 years ago

Type: Redo, selection restoring

Steps to reproduce

  1. Go to http://localhost:8125/ckeditor5-core/tests/manual/article.html.
  2. Copy two block elements.
  3. Create new empty line.
  4. Press Ctrl + V.
  5. Press Ctrl + Z.
  6. Press Ctrl + Y.

GIF

bug_cke5

Mgsy commented 7 years ago

Fixed

Type: Undo, incorrect data

Steps to reproduce

  1. Create new block quote.
  2. Inside the QB create the list with one nested item.
  3. Select whole list.
  4. Press Delete.
  5. Press Ctrl + Z.

GIF

bug_cke5

Edit by LukaszGudel: Undoing list removal is now correctly restoring two items without adding third one. Fixed.

Mgsy commented 7 years ago

POSSIBLY FIXED

Type: Redo, crash

Steps to reproduce

  1. Create new blockquote below other block element.
  2. Type some word.
  3. Split it.
  4. Select divided word.
  5. Press Delete.
  6. Undo.
  7. Redo.
  8. Press Backspace.

Error

ckeditor.js:5 Uncaught o: move-operation-node-into-itself: Trying to move a range of nodes into one of nodes from that range. Read more: https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/framework/guides/support/error-codes.html#move-operation-node-into-itself.
    at d._execute (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:50554)
    at x.applyOperation (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:246675)
    at n (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:107507)
    at o.move (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:108252)
    at n (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:242104)
    at t.a (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:243111)
    at _.deleteContent (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:222073)
    at _.on (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:29276)
    at _.fire (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:23735)
    at _.(anonymous function) [as deleteContent] (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:29327)

GIF

bug_cke5

Edit by LukaszGudel: I can not recreate this issue using those steps. It's possible that it is fixed but it would be better if someone confirms this.

Mgsy commented 7 years ago

FIXED

Type: Undo, incorrect data

Steps to reproduce

  1. Put the caret in the middle of some word.
  2. Press Enter.
  3. Select divided parts.
  4. Press Delete.
  5. Add the block quote.
  6. Undo twice.

Current result

Divided word is missing bottom part.

GIF

bug_cke5

Edit by LukaszGudel: Undoing is correctly restoring both lines. Fixed.

Mgsy commented 7 years ago

POSSIBLY FIXED

Type: Undo, crash

Steps to reproduce

  1. Go to http://localhost:8125/ckeditor5-core/tests/manual/article.html.
  2. Put the caret at the end of the Heading 1.
  3. Press Enter.
  4. Press Backspace.
  5. Select Heading 1.
  6. Press Delete
  7. Undo twice.

Current result

Editor crashes.

GIF

bug_cke5

Error

Uncaught o: model-nodelist-offset-out-of-bounds: Given offset cannot be found in the node list. Read more: https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/framework/guides/support/error-codes.html#model-nodelist-offset-out-of-bounds.
    at i.offsetToIndex (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:93655)
    at l.offsetToIndex (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:13267)
    at d.get index [as index] (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:1937)
    at d.get textNode [as textNode] (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:2013)
    at d.get nodeBefore [as nodeBefore] (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:2196)
    at h.getSelectedElement (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:73188)
    at _updateCaptionVisibility (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:390795)
    at _.listenTo (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:390524)
    at _.fire (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:23714)
    at _.(anonymous function) [as render] (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:29306)

Edit by LukaszGudel: I can not recreate this issue using those steps. It's possible that it is fixed but it would be better if someone confirms this.

Mgsy commented 7 years ago

FIXED

Type: undo, incorrect data

Steps to reproduce

  1. Type test.
  2. Put the caret after t and press Enter.
  3. Repeat step 2 for every character. You should receive: 
    t
e
s
t
  4. Delete every character, starting from the last one.
  5. Put the caret at the last empty line.
  6. Press Backspace 3 times.
  7. Undo until word is restored.

Current result

Word has been restored, but characters have incorrect position - the result is tets, instead of test.

GIF

bug_cke5

Edit by LukaszGudel: word is correctly restored with correct position of characters.

Mgsy commented 7 years ago

Fixed

Type: Undo, incorrect data

Steps to reproduce

  1. Create two paragraphs with some words.
  2. Put the caret at the second paragraph.
  3. Transform it into the block quote.
  4. Select the word inside the block quote.
  5. Press Backspace 3 times.
  6. Undo.

Current result

Deleted character from the first paragraph has been restored inside the block quote.

GIF

bug_cke5

Edit by LukaszGudel: Words are correctly restored. Fixed.

Mgsy commented 7 years ago

POSSIBLY FIXED

Type: Undo, crash

Steps to reproduce

  1. Create two paragraphs with some words.
  2. Put the caret at the second paragraph.
  3. Transform it into the Heading 1.
  4. Select this word.
  5. Press Backspace 3 times.
  6. Undo.

Current result

Editor crashes.

GIF

bug_cke5

Edit by LukaszGudel: I can not recreate this issue using those steps. It's possible that it is fixed but it would be better if someone confirms this.

Mgsy commented 7 years ago

POSSIBLY FIXED

Type: Undo, crash

Steps to reproduce

  1. Select two block elements.
  2. Click on the block quote icon.
  3. Press Delete.
  4. Click on the block quote icon.
  5. Undo twice.

Current result

Editor crashes

GIF

bug_cke5

Error

ckeditor.js:5 Uncaught o: move-operation-position-invalid: Source position or target position is invalid. Read more: https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/framework/guides/support/error-codes.html#move-operation-position-invalid.
    at i._execute (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:49878)
    at x.applyOperation (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:246824)
    at o._undo (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:202841)
    at Array.editor.document.enqueueChanges (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:336109)
    at x.enqueueChanges (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:247377)
    at o.execute (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:336076)
    at on (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:29255)
    at o.fire (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:23714)
    at o.(anonymous function) [as execute] (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:29306)
    at o.execute (http://192.168.20.68:8125/ckeditor5-build-classic/tests/manual/ckeditor.js:75:220184)

Edit by LukaszGudel: I can not recreate this issue using those steps. It's possible that it is fixed but it would be better if someone confirms this.

Mgsy commented 7 years ago

Fixed

Type: Undo, incorrect data

Steps to reproduce

  1. Go to http://localhost:8125/ckeditor5-core/tests/manual/article.html.
  2. Put the caret at the end of the Heading 1.
  3. Press Enter.
  4. Press Backspace.
  5. Delete the word character by character with Backspace.
  6. Undo.

Current result

First character of the word has been moved to the new line.

GIF

bug_cke5

Edit by LukaszGudel: Text in heading is correctly restored. Fixed.

Mgsy commented 7 years ago

POSSIBLY FIXED

Type: Undo, crash

Steps to reproduce

  1. Create 3 paragraphs with some text.
  2. Select first paragraph and press Delete twice.
  3. Select first remained paragraph and part of the second one and press Delete.
  4. Transform remained paragraph into the list.
  5. Undo 3 times.

Current result

Editor crashes.

GIF

bug_cke5

Error

ckeditorerror.js:46 Uncaught CKEditorError: model-position-path-incorrect: Position path must be an array with at least one item. Read more: https://ckeditor5.github.io/docs/nightly/ckeditor5/latest/framework/guides/support/error-codes.html#model-position-path-incorrect.
 {"path":[]}
    at new Position (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:237:10)
    at addTransformationCase (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:47295:24)
    at Object.transform (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:31648:23)
    at Object.transformDeltaSets (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:31851:34)
    at Document.transformDeltas (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:42588:77)
    at UndoCommand._undo (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:36858:37)
    at Array.editor.document.enqueueChanges (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:55081:30)
    at Document.enqueueChanges (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:42470:30)
    at UndoCommand.execute (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:55080:24)
    at UndoCommand.on (http://192.168.20.68:8125/ckeditor5-core/tests/manual/article.js:5873:32)

Edit by LukaszGudel: I can not recreate this issue using those steps. It's possible that it is fixed but it would be better if someone confirms this.

Mgsy commented 7 years ago

FIXED

Type: Undo, incorrect data

Steps to reproduce

  1. Create 3 paragraphs with some text.
  2. Select first paragraph and press Delete twice.
  3. Select remained paragraphs.
  4. Press Delete.
  5. Undo twice.

Current result

The word from the second paragraph has been moved to the third paragraph.

GIF

bug_cke5

Edit by LukaszGudel: paragraphs are correctly restored.

Mgsy commented 7 years ago

FIXED

Type: Undo, selection restoring, incorrect data

Steps to reproduce

  1. Create two paragraphs with some words.
  2. Put the caret inside the word from the second paragraph.
  3. Press Backspace twice.
  4. Put the caret at the end of the first paragraph.
  5. Press Delete.
  6. Repeat step 4.
  7. Press and hold Backspace to delete whole word.
  8. Undo 3 times.

Current result

Characters deleted in step 3 appeared in the first paragraph.

GIF

bug_cke5

Edit by LukaszGudel: Paragraphs are correctly restored. Fixed

scofalik commented 7 years ago

I have a feeling that many new errors connected with Undo are regressions after a change in deleteContents we merged very recently. I am sure that at least one of them is, as I tested it. This means that we will have to solve this in OT, or revert the change.

Reinmar commented 7 years ago

Could you report this in the engine and check it there?

scofalik commented 7 years ago

Type: Redo, incorrect data

Steps to reproduce

  1. Create content with three <p>s.
  2. Select whole content and wrap it in blockquote.
  3. Put caret in the middle paragraph.
  4. Unwrap it from blockquote.
  5. Undo, undo.
  6. Redo, redo

Current result

The unwrapped paragraph is moved to the beginning.

GIF

oct-27-2017 15-22-35

Edit by LukaszGudel: Current behaviour is slightly different. CKE5-undo-redo-3-blockquotes

Mgsy commented 7 years ago

FIXED

Type: Redo, selection restoring

Steps to reproduce

  1. Create three block quotes.
  2. Select them.
  3. Press Delete.
  4. Undo until they disappear.
  5. Redo to restore removed block quotes.

Current result

Block quotes have been restored in reversed order.

GIF

bug_cke5

Edit by LukaszGudel: Blocks are restored in correct order.

Mgsy commented 7 years ago

Fixed

Type: Undo, incorrect data

Steps to reproduce

  1. Go to http://localhost:8125/ckeditor5-core/tests/manual/article.html.
  2. Put the caret after H at Heading 1.
  3. Split the word with Enter.
  4. Put the caret at the end of the second divided part.
  5. Press Backspace.
  6. Repeat step 4 and 5 for the first divided part.
  7. Put the caret at the end of the second divided part.
  8. Press Backspace 8 times.
  9. Undo until word is restored.

Current result

Word has been restored incorrectly.

GIF

bug_cke5

Edit by LukaszGudel: Characters in heading are correctly restored.

Mgsy commented 6 years ago

Fixed

Type: Undo, incorrect data

Steps to reproduce

  1. Go to http://localhost:8125/ckeditor5-core/tests/manual/article.html.
  2. Start the selection from the end of the OL List item 2 and drag it to the first Quote.
  3. Apply the block quote.
  4. Undo.

Current result

Whole block quote has disappeared.

GIF

bug_cke5

Edit by Lukasz Gudel: Content looks the same as before applying block quote. There is no content lost. Fixed.

Mgsy commented 6 years ago

POSSIBLY FIXED

Type: Undo, crash

Steps to reproduce

  1. Go to http://localhost:8125/ckeditor5-core/tests/manual/article.html.
  2. Delete whole content.
  3. Create the block quote with two empty paragraphs.
  4. Put the caret at the first paragraph.
  5. Click on the block quote icon.
  6. Undo.

Current result

Editor crashes

Error

mapper.js:398 Uncaught TypeError: Cannot read property 'is' of undefined
    at Mapper._findPositionIn (mapper.js:398)
    at Mapper.on (mapper.js:73)
    at Mapper.fire (emittermixin.js:269)
    at Mapper.toViewPosition (mapper.js:237)
    at ModelConversionDispatcher.<anonymous> (model-selection-to-view-converters.js:86)
    at ModelConversionDispatcher.fire (emittermixin.js:269)
    at ModelConversionDispatcher.convertSelection (modelconversiondispatcher.js:253)
    at Model.EditingController.listenTo (editingcontroller.js:126)
    at Model.fire (emittermixin.js:269)
    at Model._runPendingChanges (model.js:209)

Notes

Here is the first bad commit - https://github.com/ckeditor/ckeditor5-image/commit/7a1ab67059127a9b8759d5a106738907ae14c84d

GIF

bug_cke5

Edit by LukaszGudel: I can not recreate this issue using those steps. It's possible that it is fixed but it would be better if someone confirms this.

scofalik commented 6 years ago

FIXED

Type: Undo, incorrect data

Steps to reproduce

  1. Open http://localhost:8125/ckeditor5-core/tests/manual/article.html
  2. Place caret at the end of the second line.
  3. Press and keep backspace to delete text up to the beginning of the document.
  4. Press Undo.

Current result

The content is different than at the beginning

Expected result

After undo the content is same as before deleting the text

Edit by LukaszGudel: Content is the same as before deleting the text. Fixed.

CKEditorBot commented 1 year ago

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 be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

CKEditorBot commented 1 year ago

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).