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.

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

Merged
merged 6 commits into from Apr 13, 2016
Merged

WIP - Allow both bbcode and markdown #221

merged 6 commits into from Apr 13, 2016

Conversation

jayroh
Copy link
Member

@jayroh jayroh commented Apr 13, 2016

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.

else
HTML::Pipeline::MarkdownFilter
end
HTML::Pipeline::SanitizationFilter::WHITELIST
Copy link
Member Author

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.

Copy link
Collaborator

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.

Copy link
Member Author

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 :)

Copy link
Member Author

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.
@jayroh
Copy link
Member Author

jayroh commented Apr 13, 2016

I think this is ready for another review.

If/when you guys are ok with it I'll squash and rebase before merging.

@glebm glebm merged commit 119cb9b into master Apr 13, 2016
@glebm
Copy link
Collaborator

glebm commented Apr 13, 2016

I've just clicked the merge button without realizing this hasn't been rebased yet 😭
No big deal though!

@jayroh
Copy link
Member Author

jayroh commented Apr 13, 2016

¯_(ツ)_/¯ Enh, no big deal.

@glebm glebm deleted the jro-streamline-pipeline branch April 14, 2016 18:02
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.

None yet

3 participants