Opened 7 years ago

Closed 7 years ago

#9185 closed Bug (invalid)

Inline javascript strings containing html become corrupt on edit

Reported by: Paul Aumer-Ryan Owned by:
Priority: Normal Milestone:
Component: Core : Parser Version:
Keywords: Cc: Sa'ar Zac Elias

Description

Related to ticket:7243, if my editor content contains protected elements (a, area, img, or input) embedded in a javascript string, ckEditor will try to protect their attributes (href, src, name), even if their quote delimiters are escaped.

For example: ckEditor will change this:

  var link = "<a href=\"http://google.com\">Google</a>";

to this:

  var link = "<a  data-cke-saved-href=\"http://google.com\">Google href=\"http://google.com\">Google</a>";

Fix: the protectAttributes() parser (in htmldataprocessor/plugin.js) should ignore attributes with escaped delimiters. More specifically, line 291-292 (v3.6.4) should change from this:

	var protectElementRegex = /<(a|area|img|input)\b([^>]*)>/gi,
		protectAttributeRegex = /\b(on\w+|href|src|name)\s*=\s*(?:(?:"[^"]*")|(?:'[^']*')|(?:[^ "'>]+))/gi;

to this:

	var protectElementRegex = /<(a|area|img|input)\b([^>]*)>/gi,
		protectAttributeRegex = /\b(on\w+|href|src|name)\s*=\s*(?:(?:"[^"]*")|(?:'[^']*')|(?:[^ "'\\>]+))/gi;

(I just added an escaped backslash to that last non-capturing group in the regex.)

I have tested this change on the following permutations, and the regex still correctly captures all but the last two links (which should be ignored because they are invalid anyway):

<a href="http://google.com">Google</a>
<a href=http://google.com>Google</a>
<a href= 'http://google.com'>Google</a>
<a href="">Google</a>
<a href= \\>Google</a>
<a href=\"http://google.com\">Google</a>

Thanks guys!

Change History (5)

comment:1 Changed 7 years ago by Paul Aumer-Ryan

Just a clarification that I haven't tested this in a production environment--I only tested the regex on the sample strings in a debug environment (outside of ckeditor). It may need a bit more work to cooperate with the rest of the ckeditor data processing!

comment:2 Changed 7 years ago by Jakub Ś

Status: newpending

Sorry but I could not get result you are having.

If you paste

<script type="text/javascript">
  var link = "<a href=\"http://google.com\">Google</a>";
</script>

tags will be left untouched after you switch to source and back or you destroy and create editor (in Ajax sample).

What have I missed?

comment:3 Changed 7 years ago by Paul Aumer-Ryan

You're right, escaped strings within script tags are left untouched. Thanks for clarifying that, I think that's correct behavior.

However, I have a specific use case that I was trying to generalize with a javascript example. The part I forgot was the script tags--and the escaped strings get destroyed if they don't exist inside script tags.

Here's the specifics--and you guys will have to decide whether you want to support this use case at all. I'm using ckEditor in a Drupal 7 environment (pulled in via the WYSIWYG contrib module). I'm also using the Media contrib module to embed images (it comes with a ckEditor plugin that provides a toolbar button). The Media module embeds images as tokens that get replaced at runtime. So the actual value that gets inserted in the ckEditor field looks like this:

<div>[[{"type":"media","fid":"1","attributes":{"title":"<a href=\"http://www.google.com/\">Google</a>"}}]]</div>

It's that code that is being mangled.

Again, feel free to close this issue--I'm not positive it's best solved by modifying ckEditor. But I wanted to alert you guys to the fact that escaped strings, if they do exist somewhere outside of script tags, result in data loss.

comment:4 Changed 7 years ago by Wiktor Walc

I think the issue is much more complex than the mentioned example. Here we have a specific example of a placeholder that has some basic HTML inside (a link), which is being broken.

Ok, it can be fixed using the proposed code. But it's just a fix that will work for this particular case.

Imagine a different example:

<p>
	[[coolheader prefix="<h2>" title="foo bar" suffix="</h2>"]]</p>

If we put such thing in the source mode, CKEditor will have no idea on how to interpret it. After all [[ and ]] is just a text and CKEditor has no idea that it should leave intact the HTML inside, unless a dedicated plugin will give proper instructions. The fix for the link will not prevent CKEditor from changing the code above into:

<p>
	[[coolheader prefix="</p>
<h2>
	" title="foo bar" suffix="</h2>
<p>
	"]]</p>

(with config.entities set to false)

A more reasonable approach would be, for example, to write a dedicated plugin that:

  • in wysiwyg mode renders a valid HTML e.g.
    <div class="coolheader" data-prefix="&lt;h2&gt;" data-suffix="&lt;/h2&gt;">foo bar</div>
    
  • provides some basic editing support, e.g. to change suffix and prefix
  • converts the div tag into a valid placeholder when switching back to source

The dedicated plugin could be more generic, to handle all tags using [[ and ]].

See this comment and a comment below for a sample code snippet.

comment:5 Changed 7 years ago by Jakub Ś

Resolution: invalid
Status: pendingclosed

Since dedicated plugin is the best solution here, I'm closing this issue as invalid.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy