CKEDITOR changes script tag content

[Artur Formella]

Paste in source mode

<p>
	<script>alert('sample text. You are using <a href="http://www.fckeditor.net/">FCKeditor</a>');</script></p>

switch to WYSIWYG and back to Source. Result:

<p>
	<script>alert(&#39;sample text. You are using </script><a href="http://www.fckeditor.net/"><script>FCKeditor</script></a><script>&#39;);</script></p>

expected result: nothing changed

[Garry Yao]

The patch doesn't work, I'm attaching a new patch here.

[Frederico Caldeira Knabben]

Instead of having changes to the current event calls, it would be much nicer if we could have the parser throwing the "onCData" event, which would work for the contents of <script> and <style> elements.

[Artur Formella]

What was wrong?

[Artur Formella]

Replying to arczi:

What was wrong?

Ok, sorry. Now I know.

[Artur Formella]

Bug is somewhere else. Source code:

<p>
	<script>alert('sample text. You are using <a href="http://www.fckeditor.net/">FCKeditor</a>');</script></p>

is replaced with

<p>
	<script><![CDATA[alert('sample text. You are using ]]></script><a href="http://www.fckeditor.net/"><script><![CDATA[FCKeditor]]></script></a><script><![CDATA[');]]></script></p>

It means that editor "think" that <a href ...> in a HTML tag instead of text inside <script>.

Actual behavior:

<p>
	<script>--------------TEXT----------------</script><a href="http://www.fckeditor.net/"><script>---------</script></a><script>---</script></p>

Expected behavior:

<p>
	<script>--------------------------------TEXT--------------------------------------------------</script></p>

[Artur Formella]

Replying to arczi:

It means that editor "think" that <a href ...> in a HTML tag instead of text inside <script>.

It means that editor "think" that <a href ...> is a HTML tag instead of text inside <script>.

[Garry Yao]

Nice catch Artur! while the ideal fix would be some changes to the current html parser regexp to ignore those tags within pseudo CDATA section, inside 'script' and 'style', but...the expression would be quite hard to construct without touching the current lines of the parser logics. But don't hesitate to have a try :)

Matching any html tag parts which is not inside either <script> or <style>

[Garry Yao]

Another doable approach would be two times parsing:

1. first time extracting all pseudo CDATA section content and get stored;
2. second time perform the normal parsing and compensate the CDATA section into desired tag.

[Artur Formella]

Garry, could you look at #3407? It can solve this bug.

[Frederico Caldeira Knabben]

The approach is wrong here. The parser should not treat CDATA as a special kind of text node. It's a different node at all, so we have element, text, comment and CDATA. Te parser fires onCDATA with the raw CDATA value. The writer will simply write the CDATA with no special manipulations to the data, and no CDATA[[ ]] is to be added to the output.

[Garry Yao]

Replying to arczi:

Garry, could you look at #3407? It can solve this bug.

Thanks for the tip here, I've discussed with Fred on this few days ago, we found the CDATA parsing is still necessary since the '<style>' tag could be protected as '<script>' we will have in #3047, so the fix should be some how different than that.

[Garry Yao]

Replying to fredck:

The approach is wrong here. The parser should not treat CDATA as a special kind of text node.

Please correct me: Since CDATA is only a special kind of text node from w3c spec:

1. ​DOM Level1 said so
2. check node type of the only child of script element, it's shown as text.

Do we have real benefit of creating a 'cdata' type node presentation?

The writer will simply write the CDATA with no special manipulations to the data, and no CDATA ? is to be added to the output.

I can see rational of this.

[Frederico Caldeira Knabben]

Replying to garry.yao:

Please correct me: Since CDATA is only a special kind of text node from w3c spec:

No, CDATA is not a special kind of text node. It's programmatic interface simply inherits the text interface, but they are different. They are parsed and rendered differently.

Do we have real benefit of creating a 'cdata' type node presentation?

In any case, we are not trying to reproduce the W3C definition here. We need to fit it to our needs, were text nodes needs special processing (like entities replacement), while CDATA is written as is.

---

In any case, I've noticed that the patch is still trying to propose using the "text" class to represent the CDATA. We must instead have a dedicated class for CDATA under CKEDITOR.htmlParser.

I wanted to take the ticket ownership to come with the final solution, but I've tried applying this patch and it's not working. Can you please update your local copy an provide a new patch.

[Garry Yao]

Replying to fredck: I've provided an updated patch accordingly, the patch works, and you may take it over if you're intending to change the way CDATA is parsed within html parser.

[Frederico Caldeira Knabben]

I'll be attaching a new patch with some minor cleanup and modifications.

[Frederico Caldeira Knabben]

Fixed with [3496].