search

loopindex / ckeditor-track-changes

Track changes plugin for CKEditor, based on ICE (track changes for TinyMCE).
www.loopindex.com
Other
51 stars 55 forks source link

CKEditor's undo removes <ins> #114

Open ghnat opened 7 years ago

ghnat commented 7 years ago

There is a problem with undo functionality, not sure if this is related to lite plugin or some related project like ice. Attached are two movies, first where things work as expected, second when they don't.

How to reproduce bad behavior.

  1. Open text editor like libreoffice (I guess word will do the same), type some text in the text editor, select all, copy from libre/word and paste into ckeditor once. Now the text is e.g. <p>test</p>
  2. Go to lite's demo page, replace default text with what's stored in the clipboard (copied from libre/word).
  3. Accept changes to get rid of ins/del tags.
  4. Again copy same text from libre/word and paste several times into ckeditor. Each text is pasted as a paragraph (meaning lines are separated using <p>, not <br>).
  5. Press undo - the last pasted text is removed but what's unexpected, also all previously pasted lines are not marked as inserted - the ins element is removed and this is wrong.

To have it hebave correctly two alternative approaches are possible as I found:

  1. Instead of pasting text from libre/word to ckeditor just type the text. It won't be wrapped with a <p>tag and then combining copy/paste with undo works fine.
  2. If initially text is pasted from word (e.g. is wrapped with<p>) then instead of pasting text from libre/word paste from regular text editor like sublime that does linebreaks as <br>, not <p>.

So basically - if either you have <p> in ckeditor and paste <br>s or have <br>s in ckeditor and paste <p>s things seems to work. <br> + <br> should also work. But combining <p> in ckeditor and <p> in pasted text causes trouble.

good

bad

ktundwal commented 6 years ago

I am running into similar issue

Repro steps (also see attached recording)

  1. Insert 3 paras in editor
  2. Copy first 2 paras
  3. Enable change tracking via button on toolbar
  4. Paste copied text after para 3
  5. Ensure pasted text is highlighted as "insert" change tracking
  6. Add space within pasted text
  7. Press Ctrl-Z once to Undo

Behavior Space is remove. 'ins' tag is also deleted. See status bar below

Expected behavior Only space is expected to be removed. NOT ins tag

Note: This bug doesnt repro on editor hosted on http://www.loopindex.com/lite/demo/ When I inspect demo config, I noted following. When I apply same config to my instance, bug doesnt repro. 

config.enterMode = CKEDITOR.ENTER_BR;

ckeditor-change-tracking-bug

loopindex-demo-config

ktundwal commented 6 years ago

update: we found following. Currently working to determine if this leads to fixing the issue.

CkEditor internal code has toHtml() function that makes HTML compliant per its own DTD. There job is to move <ins> tag from block level to inline. In conditions that lead to this bug, this is not happening.

Rummukain commented 4 years ago

This issue is also causing a lot of headache for our users too. They are using the editor in our app for a lot of copy - pasting tasks, which require Undos from time to time, which corrupts the tracking data... I would really appriciate if someone have figured out a solution or workaround for this. (Toggling P-mode to BR-mode in CKEDITOR is not an option for us).