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.

Sign up for GitHub

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 allowed tags/attributes from spec in amphtml 1908162134430 #3084

Merged
merged 13 commits into from
Aug 28, 2019
+445 −16
Merged

Update allowed tags/attributes from spec in amphtml 1908162134430 #3084

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
4323eaa
Update allowed tags/attributes from spec in amphtml 1908162134430
westonruter Aug 23, 2019
c93cb0b
Keep amp-carousel on version 0.1 since 0.2 depends on experimental am…
westonruter Aug 23, 2019
32a12a3
Add tests for misc element/attribute changes in 1908162134430
westonruter Aug 23, 2019
09c3323
Add support for intrinsicsize on converted img elements; pass through…
westonruter Aug 23, 2019
0aee763
Prevent validating elements in dev mode; add support for validating m…
westonruter Aug 23, 2019
10a68a3
Remove mb_strlen() for cdata max_bytes validation since bytes not chars
westonruter Aug 23, 2019
5c577e4
Add AMP_Rule_Spec::DEV_MODE_ATTRIBUTE constant
westonruter Aug 23, 2019
0a26f81
Add dev-mode awareness to the base sanitizer
westonruter Aug 23, 2019
47df43c
Add phpdoc and improve expression readability
westonruter Aug 23, 2019
6451ab3
Remove redundant dev mode checks in AMP_Tag_And_Attribute_Sanitizer
westonruter Aug 26, 2019
651c6f5
Roll back special support for intrinsicsize attribute
westonruter Aug 26, 2019
2648d15
Refactor dev-mode methods
westonruter Aug 27, 2019
106ac09
Improve testing for dev-mode, amp-sidebar, and amp-list contents
westonruter Aug 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
190 changes: 185 additions & 5 deletions includes/sanitizers/class-amp-allowed-tags-generated.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
class AMP_Allowed_Tags_Generated {

private static $spec_file_revision = 920;
private static $spec_file_revision = 935;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where does this come from? It does not seem to be the spec version this PR refers to ( 1908162134430 ).

This is probably part of the data that is being retrieved when generating this file, but a comment as to where this is mirrored might be useful.

Copy link
Member Author

Choose a reason for hiding this comment

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

This comes from the AMP validator spec: https://github.com/ampproject/amphtml/blob/dad372c5c339b07302effbea75c28eae99a7a54e/validator/validator-main.protoascii#L17-L29

As you can see, it's not actually even used anywhere. We could remove it and the $minimum_validator_revision_required variable, as they are private.

private static $minimum_validator_revision_required = 375;

private static $descendant_tag_lists = array(
Expand Down Expand Up @@ -492,7 +492,7 @@ class AMP_Allowed_Tags_Generated {
),
'referrerpolicy' => array(),
'rel' => array(
'blacklisted_value_regex' => '(^|\\s)(components|dns-prefetch|import|manifest|preconnect|prefetch|preload|prerender|serviceworker|stylesheet|subresource|)(\\s|$)',
'blacklisted_value_regex' => '(^|\\s)(components|dns-prefetch|import|manifest|preconnect|prefetch|preload|prerender|serviceworker|stylesheet|subresource)(\\s|$)',
),
'role' => array(),
'tabindex' => array(),
Expand Down Expand Up @@ -1356,7 +1356,6 @@ class AMP_Allowed_Tags_Generated {
5,
),
),
'mandatory_ancestor' => 'form',
'requires_extension' => array(
'amp-autocomplete',
),
Expand Down Expand Up @@ -1857,6 +1856,9 @@ class AMP_Allowed_Tags_Generated {
'',
),
),
'slide' => array(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The generator should probably be adapted to use short array syntax as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, although we currently ignore the whole file from PHPCS:

amp-wp/phpcs.xml

Line 102 in c894d9a

<exclude-pattern>includes/sanitizers/class-amp-allowed-tags-generated.php</exclude-pattern>

Also, this may be made irrelevant with #2769, depending on how we decide to store the spec.

'value_regex' => '[0-9]+',
),
'type' => array(
'value' => array(
'slides',
Expand Down Expand Up @@ -1913,7 +1915,11 @@ class AMP_Allowed_Tags_Generated {
'',
),
),
'slide' => array(
'value_regex' => '[0-9]+',
),
'type' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value' => array(
'carousel',
Expand Down Expand Up @@ -1969,6 +1975,9 @@ class AMP_Allowed_Tags_Generated {
'',
),
),
'slide' => array(
'value_regex' => '[0-9]+',
),
'type' => array(
'value' => array(
'slides',
Expand Down Expand Up @@ -2039,7 +2048,11 @@ class AMP_Allowed_Tags_Generated {
'',
),
),
'slide' => array(
'value_regex' => '[0-9]+',
),
'type' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value' => array(
'carousel',
Expand Down Expand Up @@ -2323,6 +2336,7 @@ class AMP_Allowed_Tags_Generated {
'offset-seconds' => array(
'value_regex' => '-?\\d+',
),
'template' => array(),
'timestamp-ms' => array(
'value_regex' => '\\d+',
),
Expand Down Expand Up @@ -2472,6 +2486,7 @@ class AMP_Allowed_Tags_Generated {
'media' => array(),
'min' => array(),
'mode' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => array(
'overlay',
Expand Down Expand Up @@ -2669,6 +2684,7 @@ class AMP_Allowed_Tags_Generated {
'value_regex' => '[0-9]+',
),
'mode' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => array(
'overlay',
Expand Down Expand Up @@ -4796,7 +4812,6 @@ class AMP_Allowed_Tags_Generated {
'disallowed_ancestor' => array(
'amp-story',
),
'mandatory_parent' => 'body',
Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. No longer does amp-sidebar have to be directly a child of the body element. Now it is merely recommended:

<amp-sidebar> is recommended to be be a direct child of the <body> to preserve a logical DOM order (for accessibility) as well as to avoid altering its behavior by a container element. Note that having an ancestor of amp-sidebar with a set z-index may cause the sidebar to appear below other elements (such as headers), breaking its functionality.

'requires_extension' => array(
'amp-sidebar',
),
Expand Down Expand Up @@ -5723,6 +5738,118 @@ class AMP_Allowed_Tags_Generated {
'spec_url' => 'https://amp.dev/documentation/components/amp-video',
),
),
array(
'attr_spec_list' => array(
'album' => array(),
'alt' => array(),
'artist' => array(),
'artwork' => array(),
'attribution' => array(),
'autoplay' => array(
'value' => array(
'',
),
),
'controls' => array(
'value' => array(
'',
),
),
'controlslist' => array(),
'crossorigin' => array(),
'data-amp-bind-album' => array(),
'data-amp-bind-alt' => array(),
'data-amp-bind-artist' => array(),
'data-amp-bind-artwork' => array(),
'data-amp-bind-attribution' => array(),
'data-amp-bind-controls' => array(),
'data-amp-bind-controlslist' => array(),
'data-amp-bind-loop' => array(),
'data-amp-bind-poster' => array(),
'data-amp-bind-preload' => array(),
'data-amp-bind-src' => array(),
'data-amp-bind-title' => array(),
'disableremoteplayback' => array(
'value' => array(
'',
),
),
'dock' => array(
'requires_extension' => array(
'amp-video-docking',
),
),
'lightbox' => array(),
'lightbox-thumbnail-id' => array(
'value_regex_casei' => '^[a-z][a-z\\d_-]*',
),
'loop' => array(
'value' => array(
'',
),
),
'media' => array(),
'muted' => array(
'value' => array(
'',
),
),
'noaudio' => array(
'value' => array(
'',
),
),
'noloading' => array(
'value' => array(
'',
),
),
'object-fit' => array(),
'object-position' => array(),
'placeholder' => array(),
'poster' => array(),
'preload' => array(
'value' => array(
'auto',
'metadata',
'none',
'',
),
),
'rotate-to-fullscreen' => array(
'value' => array(
'',
),
),
'src' => array(
'blacklisted_value_regex' => '__amp_source_origin',
'value_url' => array(
'allow_relative' => true,
'protocol' => array(
'https',
),
),
),
),
'tag_spec' => array(
'also_requires_tag_warning' => array(
'amp-video extension .js script',
),
'amp_layout' => array(
'supported_layouts' => array(
6,
2,
3,
7,
1,
4,
),
),
'mandatory_ancestor' => 'amp-story-page-attachment',
'spec_name' => 'amp-story >> amp-story-page-attachment >> amp-video',
'spec_url' => 'https://amp.dev/documentation/components/amp-video',
),
),
array(
'attr_spec_list' => array(
'album' => array(),
Expand Down Expand Up @@ -6525,6 +6652,21 @@ class AMP_Allowed_Tags_Generated {
),
),
),
'canvas' => array(
array(
'attr_spec_list' => array(
'height' => array(),
'width' => array(),
),
'tag_spec' => array(
'mandatory_ancestor' => 'amp-script',
'requires_extension' => array(
'amp-script',
),
'spec_url' => 'https://amp.dev/documentation/guides-and-tutorials/learn/spec/amphtml/#html-tags',
),
),
),
'caption' => array(
array(
'attr_spec_list' => array(),
Expand Down Expand Up @@ -7038,6 +7180,18 @@ class AMP_Allowed_Tags_Generated {
'spec_url' => 'https://amp.dev/documentation/components/amp-image-slider',
),
),
array(
'attr_spec_list' => array(
'align' => array(),
'fetch-error' => array(
'mandatory' => true,
),
),
'tag_spec' => array(
'mandatory_ancestor' => 'amp-list',
'spec_name' => 'AMP-LIST DIV [fetch-error]',
),
),
),
'dl' => array(
array(
Expand Down Expand Up @@ -8495,7 +8649,10 @@ class AMP_Allowed_Tags_Generated {
),
),
'height' => array(),
'importance' => array(),
'intrinsicsize' => array(),
'ismap' => array(),
'loading' => array(),
'longdesc' => array(
'blacklisted_value_regex' => '__amp_source_origin',
'value_url' => array(
Expand Down Expand Up @@ -8623,6 +8780,7 @@ class AMP_Allowed_Tags_Generated {
'data-amp-bind-size' => array(),
'data-amp-bind-spellcheck' => array(),
'data-amp-bind-step' => array(),
'data-amp-bind-type' => array(),
'data-amp-bind-value' => array(),
'data-amp-bind-width' => array(),
'disabled' => array(),
Expand Down Expand Up @@ -8694,6 +8852,7 @@ class AMP_Allowed_Tags_Generated {
'data-amp-bind-size' => array(),
'data-amp-bind-spellcheck' => array(),
'data-amp-bind-step' => array(),
'data-amp-bind-type' => array(),
'data-amp-bind-value' => array(),
'data-amp-bind-width' => array(),
'disabled' => array(),
Expand Down Expand Up @@ -9472,7 +9631,7 @@ class AMP_Allowed_Tags_Generated {
'hreflang' => array(),
'media' => array(),
'rel' => array(
'blacklisted_value_regex' => '(^|\\s)(canonical|components|import|manifest|preload|serviceworker|stylesheet|subresource|)(\\s|$)',
'blacklisted_value_regex' => '(^|\\s)(canonical|components|import|manifest|preload|serviceworker|stylesheet|subresource)(\\s|$)',
'mandatory' => true,
),
'sizes' => array(),
Expand Down Expand Up @@ -10168,6 +10327,24 @@ class AMP_Allowed_Tags_Generated {
'spec_name' => 'meta name=amp-recaptcha-input',
),
),
array(
'attr_spec_list' => array(
'content' => array(
'mandatory' => true,
),
'name' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => array(
'amp-script-src',
),
),
),
'tag_spec' => array(
'mandatory_parent' => 'head',
'spec_name' => 'meta name=amp-script-src',
),
),
array(
'attr_spec_list' => array(
'content' => array(),
Expand Down Expand Up @@ -12197,6 +12374,7 @@ class AMP_Allowed_Tags_Generated {
'requires_usage' => 2,
'version' => array(
'0.1',
'0.2',
Copy link
Member Author

Choose a reason for hiding this comment

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

This change is suppressed by c93cb0b.

'latest',
),
),
Expand Down Expand Up @@ -15470,6 +15648,7 @@ class AMP_Allowed_Tags_Generated {
'font-weight' => array(),
'glyph-orientation-horizontal' => array(),
'glyph-orientation-vertical' => array(),
'height' => array(),
'image-rendering' => array(),
'kerning' => array(),
'letter-spacing' => array(),
Expand Down Expand Up @@ -15503,6 +15682,7 @@ class AMP_Allowed_Tags_Generated {
'vector-effect' => array(),
'viewbox' => array(),
'visibility' => array(),
'width' => array(),
'word-spacing' => array(),
'writing-mode' => array(),
'xml:lang' => array(),
Expand Down