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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid serving dirty AMP until it is supported by the runtime #1192

Closed
westonruter opened this issue Jun 2, 2018 · 2 comments
Closed

Avoid serving dirty AMP until it is supported by the runtime #1192

westonruter opened this issue Jun 2, 2018 · 2 comments
Milestone

Comments

@westonruter
Copy link
Member

Since dirty AMP is not supported yet (see ampproject/amphtml#15583), on native AMP sites (where there is no non-AMP version) we should go ahead and auto-accept (sanitize) validation errors to remove the invalid AMP markup from the page. This could be simply implemented via:

if ( current_theme_supports( 'amp' ) ) {
    add_filter( 'amp_validation_error_sanitized', '__return_true' );
}

However what also needs to be done is that the validation errors need to always be shown in the editor instead of suppressing them once accepted.

This would basically mean that this logic would not normally run:

https://github.com/Automattic/amp-wp/blob/90acca403917c2e7b7f1b5985b03bdee7ffc6697/includes/class-amp-theme-support.php#L1141-L1153

The invalid AMP URL page should show a special warning when in native AMP instead of what is currently displayed:

This site is configured to automatically decide whether or not some validation errors may be accepted. For these errors, you cannot change their status below.

@westonruter westonruter added this to the v1.0 milestone Jun 2, 2018
@westonruter
Copy link
Member Author

/cc @ericlindley-g

@westonruter
Copy link
Member Author

This also means we'll need to prevent AMP-specific Gutenberg blocks from being registered when not in native AMP mode.

westonruter added a commit that referenced this issue Jun 6, 2018
… handling

* Add admin settings for picking between disabled, paired, and native theme support. See #1196.
* Add checkbox for automatically allowing tree shaking.
* Add checkbox for automatically sanitizing all validation errors (including tree shaking).
* Make explicitly clear that unaccepted validation errors will block rendering on AMP
* Prevent serving dirty AMP by forcibly sanitizing all validation errors when amp_is_canonical(). See #1192.
* When validation errors are automatically sanitized, ensure the terms' term_group is updated when re-checking.
* Update AMP_Options_Manager::get_options() and to return default values.
westonruter added a commit that referenced this issue Jun 6, 2018
… handling

* Add admin settings for picking between disabled, paired, and native theme support. See #1196.
* Add checkbox for automatically allowing tree shaking.
* Add checkbox for automatically sanitizing all validation errors (including tree shaking).
* Make explicitly clear that unaccepted validation errors will block rendering on AMP
* Prevent serving dirty AMP by forcibly sanitizing all validation errors when amp_is_canonical(). See #1192.
* When validation errors are automatically sanitized, ensure the terms' term_group is updated when re-checking.
* Update AMP_Options_Manager::get_options() and to return default values.
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

No branches or pull requests

1 participant