Opened 15 years ago

Closed 15 years ago

Last modified 14 years ago

#3869 closed Bug (fixed)

Script with comment

Reported by: Artur Formella Owned by: Artur Formella
Priority: Normal Milestone: CKEditor 3.0
Component: Core : Output Data Version:
Keywords: Review+ Cc:

Description

Paste the following content in source mode, switch to WYSIWYG and back to source.

	<script language="Javascript" type="text/javascript">
	<!--
	var factive_color = '#F9F9F0';
	var faonmouse_color = '#DEE3E7';
	var faonmouse2_color = '#EFEFEF';
	var cname = 'bb038dfef17';

	//-->
	</script>

Result:

<script language="Javascript" type="text/javascript">
	<!--{cke_protected}%3C!%2D%2D%0A%09var%20factive_color%20%3D%20'%23F9F9F0'%3B%0A%09var%20faonmouse_color%20%3D%20'%23DEE3E7'%3B%0A%09var%20faonmouse2_color%20%3D%20'%23EFEFEF'%3B%0A%09var%20cname%20%3D%20'bb038dfef17'%3B%0A%0A%09%09%2F%2F%2D%2D%3E-->
	</script>

Expected result: No change

Attachments (3)

3869.patch (3.1 KB) - added by Artur Formella 15 years ago.
3869_2.patch (4.2 KB) - added by Artur Formella 15 years ago.
3869_3.patch (4.8 KB) - added by Artur Formella 15 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 15 years ago by Artur Formella

Owner: set to Artur Formella
Status: newassigned

Changed 15 years ago by Artur Formella

Attachment: 3869.patch added

comment:2 Changed 15 years ago by Artur Formella

Keywords: Review? added

comment:3 Changed 15 years ago by Garry Yao

Keywords: Review- added; Review? removed

You're always sensitive to various ways of comments ;), maybe the implementation could be further simplified by eliminate the encoding of comments inside script tag at first place instead of restore it later.

var regexes =
	[
		// Script tags will also be forced to be protected, otherwise
		// IE will execute them.
		/<script[\s\S]*?<\/script>/gi,

		// <noscript> tags (get lost in IE and messed up in FF).
		/<noscript[\s\S]*?<\/noscript>/gi,

		// We must protect all comments
		// to avoid loosing them (of course, IE related).
		/<!--(:?!{cke_protected})[\s\S]*?-->/g,

	]

comment:4 in reply to:  3 Changed 15 years ago by Artur Formella

Replying to garry.yao:

You're always sensitive to various ways of comments ;), maybe the implementation could be further simplified by eliminate the encoding of comments inside script tag at first place instead of restore it later.

No :) This bug is not only with comments but also with all protectedSource.

Changing order in regexes would fix only few cases.

More buggy content:

<?
echo 'aaa<script>bbb</script>bbbb';
?>
<!-- <? echo $code ?> -->
<? echo "<noscript>aaa</noscript>" ?>

and any combination of protectedSource inside of other protectedSource.

This content works now, but won't work with your regexes

<!--
	<script language="Javascript" type="text/javascript">
	var factive_color = '#F9F9F0';
</script>//-->

Changed 15 years ago by Artur Formella

Attachment: 3869_2.patch added

comment:5 Changed 15 years ago by Artur Formella

Keywords: Review? added; Review- removed

comment:6 Changed 15 years ago by Garry Yao

Keywords: Review- added; Review? removed

I see rational behind this now, yes, double checking the replaced string is inevitable at least for now, I still see two optimzie points:

  1. Instead of encode and decode twice, we can defer the encode after the marking is done, which means the first item we try to mark all pairs pending protected, second time we catch all of them and perform the encoding;
  2. We may further turning the sequence of the regexps list order by appearing frequency in desc;

comment:7 Changed 15 years ago by Artur Formella

  1. It would increase complexity because we would need to scan twice to protect everything, event with depth=1.

How to mark it?
I think protection is a good mark because we don't need to scan twice :)

We don't even know if it is in pairs, i.e (depth=3, order in config: comments, script, php):

aaa
<?
echo 'bbb<script>
<!-- 
ccc();
//--></script>
ddd';
?>
eee
  1. How to learn the correct order?

The same problem was in V2 and was fixed in similar way but at restoring. In V3 we have different parser and it is more elegant to do it during protection.

Changed 15 years ago by Artur Formella

Attachment: 3869_3.patch added

comment:8 Changed 15 years ago by Artur Formella

Keywords: Review? added; Review- removed

After talk with Garry I've made a new patch. Usage of encode/decodeURIComponent was reduced.

comment:9 Changed 15 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:10 Changed 15 years ago by Artur Formella

Resolution: fixed
Status: assignedclosed

Fixed with [3844]

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy