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

AO3-5693 Support HTML5 media tags #3637

Merged
merged 11 commits into from Oct 13, 2019

Conversation

elzj
Copy link
Contributor

@elzj elzj commented Sep 14, 2019

Issue

https://otwarchive.atlassian.net/browse/AO3-5693

Purpose

Adds support for audio and video tags in the same fields that allow video embeds and sets up a blacklist option in the config. Also extracts the existing sanitize transformers out into separate files for improved comprehensibility.

Testing Instructions

See issue for which attributes are allowed and which should be stripped out by the sanitizer.

spec/miscellaneous/lib/multimedia_sanitizer_spec.rb Outdated Show resolved Hide resolved
spec/miscellaneous/lib/multimedia_sanitizer_spec.rb Outdated Show resolved Hide resolved
spec/miscellaneous/lib/multimedia_sanitizer_spec.rb Outdated Show resolved Hide resolved
spec/miscellaneous/lib/multimedia_sanitizer_spec.rb Outdated Show resolved Hide resolved
spec/miscellaneous/lib/multimedia_sanitizer_spec.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/multimedia.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/multimedia.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/multimedia.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/multimedia.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/multimedia.rb Outdated Show resolved Hide resolved
@redsummernight redsummernight added the Has Production Config Changes Modifies the config file and needs special attention when deploying label Sep 14, 2019
config/config.yml Outdated Show resolved Hide resolved
def sanitized_classes
user_classes.split(" ").
select { |user_class| valid_class?(user_class) }.
join(" ")

Choose a reason for hiding this comment

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

Layout/MultilineMethodCallIndentation: Use 2 (not 13) spaces for indenting an expression spanning multiple lines.

# then rejoin them
def sanitized_classes
user_classes.split(" ").
select { |user_class| valid_class?(user_class) }.

Choose a reason for hiding this comment

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

Layout/MultilineMethodCallIndentation: Use 2 (not 13) spaces for indenting an expression spanning multiple lines.

lib/otw_sanitize/user_class_sanitizer.rb Show resolved Hide resolved
lib/otw_sanitize/media_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/media_sanitizer.rb Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/embed_sanitizer.rb Outdated Show resolved Hide resolved
Copy link
Member

@sarken sarken left a comment

Choose a reason for hiding this comment

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

Just a couple comments/questions. I haven't checked the EmbedSanitizer yet, though, so feel free to ignore them until them.

spec/miscellaneous/lib/media_sanitizer_spec.rb Outdated Show resolved Hide resolved
lib/otw_sanitize/media_sanitizer.rb Outdated Show resolved Hide resolved
spec/miscellaneous/lib/html_cleaner_spec.rb Outdated Show resolved Hide resolved
@sarken
Copy link
Member

sarken commented Sep 23, 2019

Just noting that I don't have any additional comments on EmbedSanitizer

@sarken sarken changed the title AO3-5693: Support HTML5 media tags AO3-5693 Support HTML5 media tags Oct 6, 2019
@sarken
Copy link
Member

sarken commented Oct 6, 2019

Just switching labels since this has been reviewed and needs a few tweaks.

Copy link
Member

@redsummernight redsummernight left a comment

Choose a reason for hiding this comment

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

There are comments remaining:

Otherwise looks good to me. I've collapsed the rest.

@redsummernight redsummernight merged commit bcb319b into otwcode:master Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Production Config Changes Modifies the config file and needs special attention when deploying Reviewed: Ready to Merge
Projects
None yet
4 participants