Skip to content

Make TinyMCE more tolerant about HTML elements - #3437

Merged
andy-armstrong merged 2 commits into
rc/2014-04-23from
andya/rc-fix-tinymce
Apr 23, 2014
Merged

Make TinyMCE more tolerant about HTML elements#3437
andy-armstrong merged 2 commits into
rc/2014-04-23from
andya/rc-fix-tinymce

Conversation

@andy-armstrong

Copy link
Copy Markdown
Contributor

STUD-1565

This change improves TinyMCE's handling of some HTML elements (e.g. span tags, math tags etc) but doesn't completely solve the issues our users are reporting:

http://help.edge.edx.org/discussions/problems/4718-help-me-understand-what-studio-strips-from-my-code

In particular, TinyMCE will still rewrite the user's HTML, losing whitespace, comments etc. It also loses tags which strictly speaking aren't allowed outside the head tag, but for some reason seems to be used as a workaround in most browsers. I tried all of the TinyMCE options I could find, but I think I've hit the limit now.

@cahrens Let me know whether you think this is a net win, or whether it doesn't get us far enough. Either way, we still need your changes, I believe.

@nasthagiri Could you also review this when you have a chance.

@andy-armstrong

Copy link
Copy Markdown
Contributor Author

@singingwolfboy FYI, this is the change I'd like to get in to this week's release if possible.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intended behavior for the code editor to add the "p" and "/p" tags around the img?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine (and would be impossible to prevent).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just something that TinyMCE does to clean up the HTML, so there's nothing we can do about it (I tried all the options I could find). Unfortunately this is one of the complaints we have received, because users don't like their HTML getting extra tags. It is especially bad when it decides to turn trailing whitespace into

 

.

@andy-armstrong

Copy link
Copy Markdown
Contributor Author

FYI, I had more integration tests but I had to remove them because they weren't passing and I couldn't find a way to disable them. I'll see if I can develop more this morning.

@andy-armstrong

Copy link
Copy Markdown
Contributor Author

@cahrens I've removed verify_html since you pointed out that TinyMCE dropped support for it in 4. That explains why it didn't seem to help!

@cahrens

cahrens commented Apr 23, 2014

Copy link
Copy Markdown

👍

@nasthagiri

Copy link
Copy Markdown
Contributor

👍 Although I would like to assess our overall security framework regarding 3rd party code at some point.

andy-armstrong added a commit that referenced this pull request Apr 23, 2014
Make TinyMCE more tolerant about HTML elements
@andy-armstrong
andy-armstrong merged commit 56bf7d8 into rc/2014-04-23 Apr 23, 2014
@andy-armstrong
andy-armstrong deleted the andya/rc-fix-tinymce branch April 23, 2014 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants