search

OpenESignForms / vaadin-ckeditor

Vaadin component wrapper around CKEditor
4 stars 11 forks source link

CKEditorTextField being flagged as modified without user modification #61

Closed ghost closed 7 years ago

ghost commented 7 years ago

VCKEditorTextField maintains a copy of the text sent from the server. The copy will then be used to check if editor data has been modified. However, in the following scenario, the editor performs entity conversion on setData() call and subsequently a 'change' event is fired. The field's listener compares the copy and the editor (converted) data and reports the newly converted text to the server. The component is marked as modified even when no user intervention has happened.

The solution would be to update the copy after the data is set into the editor initially. Due to my limited knowledge of how CKEditor works, my fix is not perfect to share here.

OpenESignForms commented 7 years ago

We have seen this before, but we don't consider it an error because the HTML you sent over has been changed by the editor. We don't really care if the editor changes it or a user of the editor changes it. If the HTML changes from what you gave it, it's considered modified.

Of course, you are free to have the behavior you've done, but note that editor's HTML and the server's HTML are not the same. We'll have to think about whether it's better to hide the fact that the editor's HTML no longer matches or not. Because if you save the modified HTML provided by the editor, and then later set the editor to that HTML, no such modification issues will appear because the server's HTML will match the editor's.

ghost commented 7 years ago

For security reason, I do not agree with saving whatever sent from the client (CKEditor). All HTML input are sanitised before storing in the database. The issue was raised and fixed as part of our responsible contribution back to the open-source project/community.

OpenESignForms commented 7 years ago

Your help is appreciated. I would recommend you put the editor in read only mode to avoid getting updates. But as the user can and will update the contents (under normal use as that's rather why there's an HTML editor), you'd presumably have to trust the HTML string value it provides then.

But I agree that if it gives an immediate change as a result of doing a 'CKEditorTextField.setValue()' call, you can probably safely ignore it.

OpenESignForms commented 7 years ago

We couldn't make the code changes you have work for us, but we did use the basic ideas you presented to resolve this. It's unclear that this is truly correct, but it does seem to make sense that after we set the editor with some HTML, we ignore any changes the editor emits to the HTML until a user event triggers a save/update. Thanks for the ideas!