Fix DFL-3650 - need to escape control characters before parsing with innerHTML #107

Merged
merged 3 commits into from Oct 10, 2012

Projects

None yet

4 participants

@hzr
Contributor
hzr commented Oct 8, 2012

No description provided.

@hzr hzr and 1 other commented on an outdated diff Oct 8, 2012
src/syntaxhighlight/js/tokenizer.js
+ '\u0010': '\\u0010',
+ '\u0011': '\\u0011',
+ '\u0012': '\\u0012',
+ '\u0013': '\\u0013',
+ '\u0014': '\\u0014',
+ '\u0015': '\\u0015',
+ '\u0016': '\\u0016',
+ '\u0017': '\\u0017',
+ '\u0018': '\\u0018',
+ '\u0019': '\\u0019',
+ '\u001a': '\\u001a',
+ '\u001b': '\\u001b',
+ '\u001c': '\\u001c',
+ '\u001d': '\\u001d',
+ '\u001e': '\\u001e',
+ '\u001f': '\\u001f'
hzr
hzr Oct 8, 2012 Contributor

This would obviously be nicer with a replace function, but I followed the old format to mitigate the risk of messing up something deep down in the tokenizer.

chriskr
chriskr Oct 8, 2012 Contributor

\u0009, \u000A and \u000D are valid characters in XML, they don't need to be escaped.

Has someone tried to run DFL in text/html? There are some unknown elements, but that shouldn't be really a problem technically?

Also this change would break the tooltip after such a char. Matching is done on the real source based on character count offset. An alternative approach would be to replace with a small markup snippet, e.g. for \u0004 <span data-control-char="EOT"> </span> and display that with CSS similar to how e.g. Sublime Text displays control chars, a black box with the ASCI name (content: attr(data-control-char);.

(Actually that wouldn't work either, i think.)

hzr
hzr Oct 8, 2012 Contributor

About those characters, right, thanks!

I'm not sure if it's worth special-casing these for tooltips as they will appear very rarely. But I can think of some other way of doing it.

Also, back on vacation with you! ;)

hzr
hzr Oct 9, 2012 Contributor

Tried running it as text/html now. Works mostly fine, only found minor issues with some styling and no being able to resize panels (maybe because some parent element check fails now, I dunno).

Contributor
danfooo commented Oct 8, 2012

Good!

@p01
Contributor

Note sure what's the best approach. But that will do as a quick-fix

Contributor
hzr commented Oct 9, 2012

Fixed the tooltip issue. The spans contains the control pictures as "fallback" (not that this will ever be needed).

Contributor
hzr commented Oct 9, 2012

And yes, this is totally ugly.

Contributor
danfooo commented Oct 9, 2012

Should those control pictures be used in escape_html too?

Contributor
hzr commented Oct 9, 2012

@danfooo I tried that, but that would require lots of changes to DOM editing. More than it's worth.

Contributor
danfooo commented Oct 9, 2012

Ah, I see. Seems like a good enough solution then.

Contributor
p01 commented Oct 9, 2012

Looks better.

@hzr hzr merged commit 63f57d1 into escape-control-characters-reviewed Oct 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment