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

Don't sanitize controls attribute #430

Merged
merged 1 commit into from
Sep 13, 2022
Merged

Conversation

dometto
Copy link
Member

@dometto dometto commented Sep 13, 2022

Resolves gollum/gollum#1856

Our Video and Audio Macros are rendering controls attributes in the <video> and <audio> tags. Two problems:

  1. controls was not added to Loofah's safelist. This was not a problem until we started sanitizing Macro output (in sanitize macros #402).
  2. even when added to the safelist, the controls attribute is still being sanitized due to HTML5 empty attributes are being scrubbed flavorjones/loofah#242

This PR fixes 1 by adding controls to the safelist, and 2 by using a workaround: controls="true" as opposed to just controls is not sanitized by loofah. The latter should not be necessary, and when the loofah issue is resolved we can revert that change.

@dometto dometto merged commit 351ad13 into gollum:master Sep 13, 2022
@dometto dometto deleted the fix_controls branch September 13, 2022 09:59
@dometto dometto added bug and removed bug labels Sep 13, 2022
dometto added a commit to repotag/gollum-lib that referenced this pull request Nov 27, 2022
aljungberg added a commit to aljungberg/gollum-lib that referenced this pull request Feb 9, 2023
…le).

- Using the `="true"` work-around as in gollum#430.
- The change in `sanitization.rb` further addresses gollum/gollum#1587 by allowing more of the standard video properties.
- Also featured: less escaping, capitalisation fix.
dometto pushed a commit that referenced this pull request Mar 21, 2023
* New: `<<Video ..., auto=true>>` plays clips inline on a loop (GIF style).
- Using the `="true"` work-around as in #430.
- The change in `sanitization.rb` further addresses gollum/gollum#1587 by allowing more of the standard video properties.
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.

Video Controls Missing
1 participant