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

Strip HTML comments in AMP output #2842

Closed
swissspidy opened this issue Jul 18, 2019 · 5 comments
Closed

Strip HTML comments in AMP output #2842

swissspidy opened this issue Jul 18, 2019 · 5 comments
Labels
Enhancement New feature or improvement of an existing one Sanitizers

Comments

@swissspidy
Copy link
Collaborator

This was something that was suggested in the support forums

Currently, the AMP plugin warns you when there's randomly generated content on a page. That doesn't work well with the post-processor cache.

In the case from the support forums, that randomly generated content was likely due to some HTML comments. Now the question is whether random content in HTML comments can be ignored for this purpose, e.g. by simply removing any HTML comments from the source code. That would make the cache logic more robust.

@swissspidy swissspidy added Enhancement New feature or improvement of an existing one Sanitizers labels Jul 18, 2019
@westonruter
Copy link
Member

A problem with stripping the comments is that we'd need to restore them after post-processing. Also, the removal would need to be suspended when doing validation because comments are how invalid markup is attributed to plugins/themes.

We may also need to consider whether the post processor is just not robust enough of an idea to be feasible and if it should be yanked. Better to have someone rely on a page cache.

@westonruter
Copy link
Member

See also #2694 (comment)

@luckyankit
Copy link

Is there any way, any hack using which I may strip off the HTML comments for now?

@westonruter
Copy link
Member

@AkkiVerma Where are the comments coming from in your case?

@westonruter
Copy link
Member

Closing in favor of #4455.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or improvement of an existing one Sanitizers
Projects
None yet
Development

No branches or pull requests

3 participants