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

Ensure layout attributes are only allowed on supporting elements #1075

merged 1 commit into from Apr 15, 2018


None yet
2 participants
Copy link

westonruter commented Apr 15, 2018

The whitelist sanitizer is currently incorrectly allowing layout attributes (layout, width, height, etc) on all elements. This PR ensures that the list of layout attributes are only merged into the attribute list for tag specs that actually declare they have amp_layout. Additionally, the value of the layout attribute itself is validated against the tag spec's supported_layouts.

This is a companion PR with #1064 to fix #1062.

Ensure layout attributes are only allowed on supporting elements
Add validation of layout attribute value

This comment has been minimized.

Copy link

kienstra commented Apr 15, 2018

Reviewing Now

Hi @westonruter,
Thanks for this PR, fixing the sanitizer's handling of layout attributes. I'm reviewing this now.

Copy link

kienstra left a comment


Hi @westonruter,
Thanks, this PR looks good. It's nice how it uses $layout_enum. And the allowed layout values in class-amp-allowed-tags-generated.php correspond to the AMP spec for the 5 components I checked.

* @since 1.0
* @var array
public static $layout_enum = array(

This comment has been minimized.


kienstra Apr 15, 2018


Good idea to use this $layout_enum.

@westonruter westonruter merged commit 0870f00 into develop Apr 15, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
continuous-integration/travis-ci/push The Travis CI build passed

@westonruter westonruter deleted the fix/layout-validation branch Apr 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment