-
Notifications
You must be signed in to change notification settings - Fork 385
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
Update AMP validator spec to 2008290323001 and improve i-amphtml-* sanitization #5356
Conversation
be07024
to
4871fc6
Compare
…with other required sanitizers
'class' => array( | ||
'disallowed_value_regex' => '(^|\\W)i-amphtml-', | ||
), |
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.
This is now sanitized via AMP_Style_Sanitizer
.
Plugin builds for 3d0d68a are ready 🛎️!
|
$results = array_merge( | ||
$results, | ||
$this->ampify_ruleset_selectors( $ruleset ) | ||
); |
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.
$results
would always be an empty array. no?
$results = array_merge( | |
$results, | |
$this->ampify_ruleset_selectors( $ruleset ) | |
); | |
$results = $this->ampify_ruleset_selectors( $ruleset ); |
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.
Yes, if this if
condition remains the first one to mutate $results
. The idea behind merging is that it's how the logic below amends $results
.
QA Passed ✅ When the AMP plugin is active and I create a Web Story, the expected Previously these would be stripped out. Also I created the Custom HTML block as shown before and I got the expected validation errors: And the expected output is correct: The <div title="What will come after?" data-hint="Look at the CSS!" class="testing">Test results: </div> Previously it would have been removed along with the |
Previously #5075.
./bin/amphtml-update.sh
(lando ssh -c 'bash ./bin/amphtml-update.sh vendor/amphtml'
).Changelog
select
anduse
elements inside ofamp-mega-menu
andamp-nested-menu
elements.amp-story-interactive-binary-poll
,amp-story-interactive-poll
,amp-story-interactive-quiz
, andamp-story-interactive-results
components. Allow inside ofamp-story-grid-layer
.imagesizes
andimagesrcset
attributes tolink
element.meta
tags withname
values foramp-story-generator-name
andamp-story-generator-version'
. See Add meta tags for creation tool name and version GoogleForCreators/web-stories-wp#4332 (comment).sticky
attribute toamp-ad
.entity
,entity-logo-src
, andentity-url
toamp-story
.amp-story-cta-layer
andamp-story-grid-layer
:scale-fade-down
andscale-fade-up
.amp-script
protocol forsrc
attribute foramp-list
and addsnodom
attribute toamp-script
.img
from appearing inamp-story-cta-layer
,amp-story-grid-layer
, andamp-story-page-attachment
, since these are only relevant for transformed AMP (which the AMP Optimizer outputs after validation).Sanitizing
i-amphtml-*
Validation errors are now raised for each instance of
i-amphtml-*
in CSS selectors and for each instance inclass
attributes. When status of validation error is removed, the selector or class name is removed, but other selectors in the stylesheet and other class names are retained. Fixes #771.Creating a Custom HTML block:
On the frontend, before the result would be no styles at all being on the page since
style[amp-custom]
got removed during validation. After, however, only the invalid styles and invalid class names are removed:As for the markup emitted to the frontend, notice instead of the
class
attribute getting removed entirely, only the invalidi-amphtml-testing
class name is removed:<div title="What will come after?" data-hint="Look at the CSS!">Test results: </div>
<div title="What will come after?" data-hint="Look at the CSS!" class="testing">Test results: </div>
Additionally, you may mark the two validation errors as Kept, at which point a third validation error will appear for the
style[amp-custom]
as a whole given the presence ofi-amphtml-
. This can be marked as Kept as well:As expected, the invalid styles are then retained (as see by "FAIL" successfully not being sanitized):
Details
3aa007a...2008290323001