IE does not escape attribute values properly

[Dirk]

reproduce: (in WYSIWYG mode)

editorObj.setData('<a href="#" x="&lt;a b=&quot;c&quot;/&gt;">a</a>')

In IE you will see: " _cke_saved_href="#">a

source code:

<a href="http://localhost/t/backend/#" x="<a b=">&quot; _cke_saved_href=&quot;#&quot;&gt;a</a>

instead of: a

source code:

<a href="#" x="&lt;a b=&quot;c&quot;/&gt;">a</a>

[Minh Nguyen]

This bug happen in IE when we have quote character in value of attribute.

[Garry Yao]

The "quote" is already fixed in #4683 and we should be looking to fix other entities here.

[Frederico Caldeira Knabben]

The patch works well, and I was about to give it R+. In the last minute, I've decided also testing the performance of it by loading a ​log document and checking the output generation performance. It's almost 40% slower after the patch.

We need also some performance optimization at this point. If we want to you CKEDITOR.tools.htmlEncode, we need to rewrite it, much probably avoiding having a function call chain there. Otherwise we can think about optimizing it in the writter functions directly.

I had some doubt about the real need of encoding other chars inside attributes, but this is definitely a need ​according to the XML specs, which is inherited into XHTML, but ​not in HTML.

As the breaking issue here has been already fixed with [5007], we don't have much urgency here, so I'm moving this ticket to the 3.3.

[Frederico Caldeira Knabben]

Ops, moving back to the 3.2 as this patch is also fixing another important issue: pasting is not working on some cases. For example, it's not possible to past this entire page (all browsers): ​http://www.w3.org/TR/html4/

[Garry Yao]

1. 4719_3.patch breaks test case dt/core/tools::test_htmlEncode2, specifically character quote should not be always escaped, it's only feasible when considering an attribute value context, the php ​htmlspecialchars will be a good API ref for it.

1. The current implementation of CKEDITOR.tools.htmlEncode is over complicated, it's safely enough to just escape manually five characters there, by a RegExp, without bothering DOM.

[Garry Yao]

Replying to fredck:

Ops, moving back to the 3.2 as this patch is also fixing another important issue...

The problem has been fixed with [5028] on #4683.

[Frederico Caldeira Knabben]

Replying to garry.yao:

1. 4719_3.patch breaks test case dt/core/tools::test_htmlEncode2, specifically character quote should not be always escaped, it's only feasible when considering an attribute value context, the php ​htmlspecialchars will be a good API ref for it.

Well, not a big issue. It may avoid us having a dedicated htmlAttEncode function.

1. The current implementation of CKEDITOR.tools.htmlEncode is over complicated, it's safely enough to just escape manually five characters there, by a RegExp, without bothering DOM.

The DOM trick is a nice attempt to make it work as fast as possible in all browsers. It's a nice solution, but we may change it so we avoid the functions chain.

[Frederico Caldeira Knabben]

Replying to garry.yao:

The problem has been fixed with [5028] on #4683.

At this point, let's get back to this one later.

[Garry Yao]

Please make the documentation of 'htmlEncodeAttr' better by emphasizing the escape happen in the context of a html attribute value.

[Minh Nguyen]

Fixed with [5271].

[Minh Nguyen]

and [5272].