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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Validator: support decimals for intrisic sizer dimensions #27544

Merged

Conversation

sebastianbenz
Copy link
Contributor

This also fixes hard coded image dimensions.

Fixes #27528

This also fixes hard coded image dimensions.

Fixes ampproject#27528
@amp-owners-bot
Copy link

amp-owners-bot bot commented Apr 2, 2020

Hey @ampproject/wg-caching, these files were changed:

validator/testdata/transformed_feature_tests/server_side_rendering.html
validator/testdata/transformed_feature_tests/server_side_rendering.out
validator/validator-main.protoascii

@sebastianbenz sebastianbenz merged commit db9d408 into ampproject:master Apr 2, 2020
@sebastianbenz sebastianbenz deleted the intrinsic-sizer-decimals branch April 2, 2020 18:11
@@ -4876,7 +4876,7 @@ tags: {
}
attrs: {
name: "src"
value_regex: "data:image\\/svg\\+xml;charset=utf-8,<svg height=\"\\d+\" width=\"\\d+\" xmlns=\"http:\\/\\/www\\.w3\\.org\\/2000\\/svg\" version=\"1\\.1\"\\/>|data:image\\/svg\\+xml;charset=utf-8,<svg height='100' width='300' xmlns='http:\\/\\/www\\.w3\\.org\\/2000\\/svg' version='1\\.1'\\/>"
value_regex: "data:image\\/svg\\+xml;charset=utf-8,<svg height=\"\\d+(\\.\\d+)?\" width=\"\\d+(\\.\\d+)?\" xmlns=\"http:\\/\\/www\\.w3\\.org\\/2000\\/svg\" version=\"1\\.1\"\\/>|data:image\\/svg\\+xml;charset=utf-8,<svg height='\\d+(\\.\\d+)?\' width='\\d+(\\.\\d+)?\' xmlns='http:\\/\\/www\\.w3\\.org\\/2000\\/svg' version='1\\.1'\\/>"
Copy link
Member

Choose a reason for hiding this comment

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

I just realized a way to make this more resilient to make this even more resilient to HTML mutations, perhaps the pattern should be modified like so:

value_regex: "data:image\\/svg\\+xml;charset=utf-8,<svg(\\s+(height=[\"']\\d+(\\.\\d+)?[\"']|width=[\"']\\d+(\\.\\d+)?[\"']|xmlns=[\"']http:\\/\\/www\\.w3\\.org\\/2000\\/svg[\"']|version=[\"']1\\.1[\"']))+\\s*(\\/>|></svg>)"

That would allow the order of the attributes to be modified freely. The whitespace would also be more forgiving. (It also combines the two patterns to use ['"] to represent the attributes. Granted this would allow something like width='1" causing a XML syntax error.) It also allows for a closing </svg> tag which actually should solve the problem of HTML minifiers from stripping self-closing slashes.

Regex without escaping:

data:image\/svg\+xml;charset=utf-8,<svg(\s+(height=["']\d+(\.\d+)?["']|width=["']\d+(\.\d+)?["']|xmlns=["']http:\/\/www\.w3\.org\/2000\/svg["']|version=["']1\.1["']))+\s*(\/>|></svg>)

@Gregable Gregable mentioned this pull request Apr 6, 2020
Gregable pushed a commit that referenced this pull request Apr 6, 2020
* cl/304511035 Add CSS Validation test for declarations.

* cl/305075265 Revision bump for #27544
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Elements with decimal dimensions cause AMP validation errors in i-amphtml-sizer for Optimized/Transformed AMP
4 participants