Skip to content

Ensure layout attributes are only allowed on supporting elements#1075

Merged
westonruter merged 1 commit intodevelopfrom
fix/layout-validation
Apr 15, 2018
Merged

Ensure layout attributes are only allowed on supporting elements#1075
westonruter merged 1 commit intodevelopfrom
fix/layout-validation

Conversation

@westonruter
Copy link
Copy Markdown
Member

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.

@kienstra
Copy link
Copy Markdown
Contributor

Reviewing Now

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

Copy link
Copy Markdown
Contributor

@kienstra kienstra left a comment

Choose a reason for hiding this comment

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

Approved

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(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good idea to use this $layout_enum.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Width attribute on table col elements causes validation errors

2 participants