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

Moving the selection should reset the buffer #3042

Closed
pjasiun opened this issue Jun 24, 2016 · 5 comments · Fixed by ckeditor/ckeditor5-typing#80
Closed

Moving the selection should reset the buffer #3042

pjasiun opened this issue Jun 24, 2016 · 5 comments · Fixed by ckeditor/ckeditor5-typing#80
Assignees
Labels
package:typing type:bug This issue reports a buggy (incorrect) behavior.
Milestone

Comments

@pjasiun
Copy link

pjasiun commented Jun 24, 2016

  1. open http://tests.ckeditor.dev:1030/build/amd/tests/undo/manual/undo
  2. type "aaa" in one place,
  3. move selection to another,
  4. type "bbb",
  5. move selection to another,
  6. type "ccc",
  7. Undo.

Expected: there should be 3 undo steps.

Actual: there is only one undo step.

Changing the selection should reset the buffer. That's it.

@Reinmar
Copy link
Member

Reinmar commented Jul 5, 2016

ckeditor/ckeditor5-typing#21 is another case to cover when fixing this issue.

@Reinmar
Copy link
Member

Reinmar commented Sep 21, 2016

This is very tricky, because we need to reset the buffer when selection changes... but not when that change is caused by typing. So the typing feature will need to block change buffer when they operate on the model.

@Reinmar
Copy link
Member

Reinmar commented Sep 21, 2016

Let's try this way:

  1. We add ChangeBuffer.lock() and unlock() methods. They should make the change buffer ignore everything that happens around.
  2. When the input or delete feature makes it changes, it should lock the buffer, so it isn't reset by those.
  3. Change buffer should also listen to change:attribute and change:range on the selection to reset itself when those things change. But only when it's not locked, ofc.

@maxbarnas
Copy link
Contributor

Prototype of fix for ckeditor/ckeditor5-typing#21 seems to help with this issue. In the meanwhile I'll add tests for the ticket.

@maxbarnas
Copy link
Contributor

maxbarnas commented Sep 22, 2016

Manual test and early version of integration test were just commited. Also, because this issue is tightly related to ckeditor/ckeditor5-typing#21 , there will be testing of styles (Bold and/or Italic).

There are obvious deficiencies e.g., lack of InputCommand, Bold and/or Italic. It'll be corrected soon.

@f1ames f1ames self-assigned this Mar 6, 2017
Reinmar referenced this issue in ckeditor/ckeditor5-typing Mar 9, 2017
Fix: New undo step should be created on selection change or applying an attribute. Closes #20. Closes #21.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-typing Oct 9, 2019
@mlewand mlewand added this to the iteration 9 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:bug This issue reports a buggy (incorrect) behavior. package:typing labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:typing type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
5 participants