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

Introduce transform_important_qualifiers arg to AMP_Style_Sanitizer #6589

Merged

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented Sep 2, 2021

Summary

This is cherry-picked from #6577 and is essentially amending #6528. When custom scripts are on the page, it's important that inline style rules be left as-is without being transformed into style rules. Take comment replying for example. The cancel reply link from core gets style="display:none". The style sanitizer by default converts that into a style rule :root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-cb45893 {display: none} and the style attribute will be replaced with data-amp-original-style="display:none" and class="amp-wp-cb45893". When comment-reply.js then tries to show the link by removing the the display property, nothing happens because the style has been applied with a style rule instead.

This PR adds a transform_important_qualifiers arg to AMP_Style_Sanitizer which will prevent style attributes from being converted into style rules, while also preventing the transformation of any !important qualifiers into style rules with high specificity. The latter transformation is actually the reason for the former, since specificity can only be maintained if all styles are moved to style rules.

This argument is false by default so that the valid-AMP behavior of preventing !important from appearing in the page will be preserved.

This sanitizer arg will be enabled as part of the above PR or #6546.

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@westonruter westonruter added this to the v2.2 milestone Sep 2, 2021
@westonruter westonruter requested a review from pierlon September 2, 2021 04:29
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2021

Plugin builds for 6e187e8 are ready 🛎️!

Copy link
Contributor

@pierlon pierlon left a comment

Choose a reason for hiding this comment

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

LGTM.

@westonruter westonruter merged commit 3f0146e into develop Sep 4, 2021
@westonruter westonruter deleted the add/transform-important-qualifiers-style-sanitizer-arg branch September 4, 2021 01:51
@fellyph fellyph self-assigned this Dec 13, 2021
@fellyph
Copy link
Collaborator

fellyph commented Dec 13, 2021

QA Passed

Twenty Twenty-one:
Screen Shot 2021-12-13 at 17 57 54

Twenty-Twenty:
Screen Shot 2021-12-13 at 18 01 08

Twenty nineteen:
Screen Shot 2021-12-13 at 17 54 06

Twenty Seventeen:
Screen Shot 2021-12-13 at 17 42 52

Twenty Sixteen:
Uploading Screen Shot 2021-12-13 at 17.54.56.png…

Twenty Fifteen:
Screen Shot 2021-12-13 at 17 52 24

Twenty Fourteen:
Screen Shot 2021-12-13 at 17 49 10

Twenty Thirteen:
Screen Shot 2021-12-13 at 17 59 28

Twenty eleven:
Screen Shot 2021-12-13 at 17 47 31

Twenty ten:
Screen Shot 2021-12-13 at 17 56 04

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bento Changelogged Whether the issue/PR has been added to release notes. CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants