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

Read data from source #89

Open
Nunivenkatarao opened this issue Feb 6, 2020 · 8 comments
Open

Read data from source #89

Nunivenkatarao opened this issue Feb 6, 2020 · 8 comments
Labels
status:confirmed An issue confirmed by the development team. type:bug A bug.

Comments

@Nunivenkatarao
Copy link

I have added some html content in source and when i clicking on submit i am able to read previous data not the one which i have added in source

Once i am coming from source to wysiwyg then only it is reading the data
is there any solution for this

@f1ames
Copy link
Contributor

f1ames commented Feb 6, 2020

Hi @Nunivenkatarao,

I see it works as you described in our From sample but we have to check if it is an issue with the sample code or component itself (did you encountered this issue in some different case/situation)?

As for CKEditor 4 itself, when the Source Mode is active, editor.getData() returns valid, up-to-date content. However, there is one important thing to remember - since you are able to type anything in the source mode (data is processed and filtered while switching back to WYSIWYG mode), allowing user to submit it may pose a security issue. So at least you need to add some content sanitisation mechanism on server side or allow users to submit data only in WYSIWYG mode.

@f1ames
Copy link
Contributor

f1ames commented Feb 6, 2020

Extracted #90.

@f1ames f1ames added status:confirmed An issue confirmed by the development team. type:bug A bug. and removed status:pending labels Mar 4, 2020
@f1ames
Copy link
Contributor

f1ames commented Mar 4, 2020

If we would like to have consistent behavior with CKEditor 4, then while in the source mode entire/up-to-date HTML should be returned (with all modifications done in source mode included.

OTOH as mentioned in #89 (comment), such behavior poses some security issues and needs additional attention on integrator side so I'm not 100% sure if we should go this way.

Any ideas on this? cc @jacekbogdanski @Dumluregn @Comandeer 


Related to ckeditor/ckeditor4#3470 and ckeditor/ckeditor4#2326.

@Dumluregn
Copy link
Collaborator

I can't really see the reason why our integration should behave differently than the CKEditor 4 itself 🤔 So I would change it to work the same way, i.e. return actual content of source area.
And on top of that let's investigate if introducing filtering as requested in ckeditor/ckeditor4#3470 wouldn't be too complicated and if not then do it.

@jacekbogdanski
Copy link
Member

jacekbogdanski commented Mar 5, 2020

We discussed lately that it may be a good idea to encode HTML in source mode also (usingeditor.getData) API. Seems like both ckeditor/ckeditor4#3470 ckeditor/ckeditor4#2326 issues are actually the same feature request, is that right?

Nevertheless, why do we connect this issue with the one about source mode? They seem not related. If I correctly understand OP issue, Angular model binding is not updated on the source mode switch, which seems like easy to fix mistake. Ach, sorry, I see it's working correctly, just sample is probably broken.

If we would like to have consistent behavior with CKEditor 4, then while in the source mode entire/up-to-date HTML should be returned (with all modifications done in source mode included.

Could you tell what's wrong here?
source

It seems to work correctly, source mode returns updated HTML. The issue with the submit button is just a minor issue of the sample.

@f1ames
Copy link
Contributor

f1ames commented Mar 9, 2020

It seems to work correctly, source mode returns updated HTML. The issue with the submit button is just a minor issue of the sample.

@jacekbogdanski are you sure about this? Form demo uses <ckeditor> ngModel internally. And its value should be always consistent with the output of editor.getData() which is not true in this case. I suspect there might be an issue with some internal data synchronization in the integration module.

@jacekbogdanski
Copy link
Member

I didn't really investigate the issue with the submit button, I just don't see how it's related to the linked issues with sourcemode encoding.

But indeed, the issue may be caused by integration itself. AFAIK model update is triggered on the editor change event, which is not called in source mode - so it may create some desync issues.

@f1ames
Copy link
Contributor

f1ames commented Mar 9, 2020

I didn't really investigate the issue with the submit button, I just don't see how it's related to the linked issues with sourcemode encoding.

That's fine, it is related in a way that when it get fixed (so there will be no issue wtih model syncing in source mode), it will be also visible there. Now it's not visible because model is not updated in source mode. So by fixing this issue we enable (and make visible) the behaviour which is discussed in ckeditor/ckeditor4#3470 and ckeditor/ckeditor4#2326.

Still treating it as an upstream is fine for me, seems like a proper way to approach this, so we should focus only on model syncing here 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

4 participants