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

Auto-link undo correction #12447

Closed
kbinieda opened this issue Sep 8, 2022 · 2 comments · Fixed by #12460
Closed

Auto-link undo correction #12447

kbinieda opened this issue Sep 8, 2022 · 2 comments · Fixed by #12460
Assignees
Labels
intro Good first ticket. squad:collaboration Issue to be handled by the Collaboration team. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@kbinieda
Copy link
Contributor

kbinieda commented Sep 8, 2022

📝 Provide detailed reproduction steps (if any)

  1. type "https://ckeditor.com abc" in content (hyperlink should be created for ckeditor.com, while "abc" still remains as text, which is fine)
  2. slowly (one character at a time) hit backspace to remove " abc" text (including space).
  3. Instead of removing space, editor undo character removal to initial "https://ckeditor.com abc" situation.

✔️ Expected result

just "https://ckeditor.com" in editor

❌ Actual result

"https://ckeditor.com abc" in editor

@kbinieda kbinieda added squad:collaboration Issue to be handled by the Collaboration team. type:bug This issue reports a buggy (incorrect) behavior. labels Sep 8, 2022
@kbinieda kbinieda self-assigned this Sep 8, 2022
@kbinieda kbinieda added the intro Good first ticket. label Sep 8, 2022
@CKEditorBot CKEditorBot added the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Sep 8, 2022
@kbinieda
Copy link
Contributor Author

@arkflpc @scofalik While working with the task, I've found that in various places requestUndoOnBackspace function is enclosed in enqueueChange while in some other it is not. Is there a reason to that?

autolink.js
model.enqueueChange( () => { deletePlugin.requestUndoOnBackspace(); } );

automediaembed.js
editor.plugins.get( 'Delete' ).requestUndoOnBackspace();

@arkflpc
Copy link
Contributor

arkflpc commented Sep 14, 2022

requestUndoOnBackspace should be called after current Batch is applied. So if it happens inside model.change() or mode.enqueueChange(), it has to be wrapped with another model.enqueueChange() to delay its execution to another Batch. If not, it's not needed.

scofalik added a commit that referenced this issue Sep 15, 2022
Fix (link): Fixed incorrect behavior when text before auto-linked url was removed by pressing backspace. Previously, the removed text was reinserted on backspace press. Closes #12447.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Sep 15, 2022
@CKEditorBot CKEditorBot added this to the iteration 57 milestone Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
intro Good first ticket. squad:collaboration Issue to be handled by the Collaboration team. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants