Add new accessibility sanitizer#4535
Merged
westonruter merged 10 commits intodevelopfrom Apr 10, 2020
Merged
Conversation
pierlon
suggested changes
Apr 8, 2020
Contributor
pierlon
left a comment
There was a problem hiding this comment.
You forgot the most important part, adding it to the list of sanitizers to be run:
--- includes/amp-helper-functions.php (revision f0c97bbfaddbc59f44e0c9508aa63d1be367a912)
+++ includes/amp-helper-functions.php (date 1586320714141)
@@ -1220,6 +1220,7 @@
'AMP_Style_Sanitizer' => [],
'AMP_Meta_Sanitizer' => [],
'AMP_Layout_Sanitizer' => [],
+ 'AMP_Accessibility_Sanitizer' => [],
'AMP_Tag_And_Attribute_Sanitizer' => [], // Note: This whitelist sanitizer must come at the end to clean up any remaining issues the other sanitizers didn't catch.
];
westonruter
approved these changes
Apr 10, 2020
pierlon
approved these changes
Apr 10, 2020
westonruter
added a commit
that referenced
this pull request
Apr 10, 2020
* Add interfaces for ARIA roles * Use Role interface in existing transformers * Add new a11y sanitizer * Port MarkupComparison trait over to AmpProject\AmpWP * Use MarkupComparison trait instead of copy-pasted methods * Add tests for new a11y sanitizer * Fix tab/space code style issue * Actually include the new sanitizer in sanitization * Add test for missing both attrs, and preserving tabindex Co-authored-by: Weston Ruter <westonruter@google.com>
amedina
reviewed
Apr 14, 2020
| public function get_sanitize_test_data() { | ||
| return [ | ||
| 'valid markup remains unchanged' => [ | ||
| '<div href="#" on="tap:some_id.toggleClass(some_class)" role="button" tabindex="0"></div>', |
Member
There was a problem hiding this comment.
@schlessera The validator complains against href="#"; it is not valid markup.
Member
|
Would like to validate the acceptance criteria here. These HTML blocks:
Should yield the corresponding
|
Contributor
|
This looks good in my local. On copying the Custom HTML markup above verbatim into Custom HTML blocks: <div on="tap:myfunction()">
None in
</div>
<div on="tap:myfunction()" tabindex="1">
Tabindex in
</div>
<div on="tap:myfunction()" role="button">
Role in
</div>...the
|
Member
|
Thanks @kienstra ; reason was missing |
westonruter
added a commit
that referenced
this pull request
Apr 15, 2020
* tag '1.5.3': Bump 1.5.3 Bump version to 1.5.3-RC1 Fix handling of Mustache templates (#4583) Stub request based on test scenario (#4588) Update tests after block-library/style.css changes in Gutenberg 7.9 (#4579) Restrict doing plugin upgrade routine when not in admin (#4538) Add new accessibility sanitizer (#4535) Fix unit tests (#4564) Add button into Site Health to reenable CSS transient caching (#4522) Restore unification of multi-page post content in Reader mode (#4547) Prevent styles from being removed when in Customizer preview with Standard mode (#4553) Omit Jetpack from being activated during PHPUnit test runs (#4474) Mock Facebook embed tests (#4474) Mock Imgur embed tests (#4474) Use title case for Paired Browsing link in edit post screen (#4540) Ensure that validation query vars persist through redirects (#4544) Add requirements to plugin file header (#4543) Force status code of validation responses to be 200 (#4533) Update optimizer test specs (#4527) Bump 1.5.3-alpha
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR includes the following changes:
AMP_Accessibility_Sanitizerclass that addsroleandtabindexattributes to elements with theonAMP event.MarkupComparisontrait from theamp/commontest suite and replaces copy-pastedassertEqualMarkup()methods throughout tests with the new trait.Roleinterface to define and document ARIA roles and use the provided constants instead of hard-coded strings in existing sanitizers/transformers that define ARIA roles.Fixes #4528
Checklist