-
Notifications
You must be signed in to change notification settings - Fork 216
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.
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
WIP - Allow both bbcode and markdown #221
Conversation
else | ||
HTML::Pipeline::MarkdownFilter | ||
end | ||
HTML::Pipeline::SanitizationFilter::WHITELIST |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking that this:
def sanitize_whitelist
HTML::Pipeline::SanitizationFilter::WHITELIST[:elements] += %w(
fieldset
legend
blockquote
)
HTML::Pipeline::SanitizationFilter::WHITELIST
end
... could definitely be better. I'm just tired and my brain is fried.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blockquote
is already in the whitelist, and it doesn't seem that fieldset/legend should be whitelisted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blockquote is already in the whitelist, and it doesn't seem that fieldset/legend should be whitelisted.
I think you're right. Markdown will just wrap a blockquote, right? If that's the case then, yeah - we should retain some parity there. Will inspect this html once I submit :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. blockquote
!
Let the sanitizer remove the fieldsets and legends and stick with just blockquotes.
I think this is ready for another review. If/when you guys are ok with it I'll squash and rebase before merging. |
I've just clicked the merge button without realizing this hasn't been rebased yet 😭 |
¯_(ツ)_/¯ Enh, no big deal. |
Instead of asking messageboard administrators to decide which "filter" to use for posts just let them use a subset of bbcode and markdown simultaneously.
TODO: move migration from default generated location into the rolled up migration, and the 0.2.0 -> 0.3.0 migration.