Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable TinyMCE sanitization when setting HTML content #4304

Merged
merged 1 commit into from
Jan 14, 2018
Merged

Enable TinyMCE sanitization when setting HTML content #4304

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion blocks/editable/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ export default class Editable extends Component {
}

content = renderToString( content );
this.editor.setContent( content, { format: 'raw' } );
this.editor.setContent( content );
Copy link
Member

Choose a reason for hiding this comment

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

Could this have other consequences? cc @iseulde

Copy link
Member

Choose a reason for hiding this comment

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

Previous discussion: #4205 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Another previous discussion: #667 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I don't really see a problem with this if we want the content to be validated. With the raw setting, we assume the content is valid and exactly as it needs to be set in the contentEditable field. I think it's a bit weird though to be getting the raw content (through parsing the nodes) and setting the content with filters and validation.

Copy link
Member

Choose a reason for hiding this comment

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

In other words, without the user changing anything about the block, the block attributes will have changed?

Copy link
Member

Choose a reason for hiding this comment

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

Why does this not happen with a normal list block? Why only a reusable list block? Can it not behave in the same way?

Copy link
Member Author

@noisysocks noisysocks Jan 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This bug happens with normal list blocks too—it's just harder to come across. For example:

  1. Create a new post
  2. Create a list block with a few items
  3. Save the post as a draft
  4. Refresh the page
  5. Add a new item to the list block
  6. Click Undo

If we prefer to always set raw content, I outlined some ways we could fix how we parse the list HTML in #4205 (comment).

}

getContent() {
Expand Down