Fix html export with ep_headings2 #33
Conversation
|
eslint fixes please :) |
ilmartyrk
force-pushed
the
html_export_fix
branch
from
dcde480 to
0e99eb8
Compare
ilmartyrk
force-pushed
the
html_export_fix
branch
from
0e99eb8 to
02f7ba6
Compare
|
@JohnMcLear sorry about that, fixed now, also went over tests again, there was some kind of a syntax diff between my local tests and here, anyway this pull will work together with this ether/ep_headings2#38 |
| if (heading.indexOf('style=') === -1) { | ||
| context.lineContent = context.lineContent.replace('>', ` style='text-align:${align}'>`); | ||
| } else { | ||
| context.lineContent = context.lineContent.replace('style=', `style='text-align:${align} `); |
There was a problem hiding this comment.
The content isn't being escape here any more dude??? Isn't that a problem? 👍
There was a problem hiding this comment.
If you check ep_headings2 plugin it isn't escaped there too as it will remove any html added by other plugins. Probably one way to fix would be to check line with indexOf('script') > -1' and if does then use Security.escapeHTML. Would that be acceptable?
There was a problem hiding this comment.
hrm, I'm sure at some point we addressed this.. script wouldn't suffice, too easy to manipulate...
There was a problem hiding this comment.
I don't see other plugins using Security.escapeHTML in getLineHTMLForExport:
https://github.com/ether/ep_font_color/blob/a3760585b9970df4d8562e8df1aea6fe3587ed91/exportHTML.js#L21
https://github.com/ether/ep_font_size/blob/e5318a81dd8b035e1cc1aa1fe5c905156fc65e2d/exportHTML.js#L18
I am not saying it shouldn't but just looks weird if one plugin does and others don't. Maybe it would be better to find some solution after hook call in main project.
|
FWIW this plugin used to do it: https://github.com/ether/ep_align/pull/33/files#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346L38 There will be a reason I did that 👍 Can't you escape plugins such as font family/size/align content and not escape in ep_headings or?? I guess the issue is if you have a color wthin an alignment? Tricky one ;( Does the alert example actually work? I'm only assuming it's a security issue, there might be something fancy going on that stops it being an issue....?? |
|
@JohnMcLear I played around with it and only way I could insert any script was through using
|
|
Perfect, let's go! |
When using together with ep_headings2 the ep_headings currently replace existing text formatting, but this fix together with ep_headings2 fix will allow headings to be aligned with valid HTML.