Send entire templates in Reader/Classic mode through the sanitizers

[westonruter]

Currently only content (and the featured image) are sent through the sanitizers. This is the classic approach, and it is why there's the content in amp_content_sanitizers. However, limiting the sanitizers to the content has some problems which would be fixed by sanitizing the entire page:

1. Markup that is output outside the content (such as in amp_post_template_head or amp_post_template_footer) is not sent through the sanitizers, resulting in the possibility of invalid AMP pages when plugins don't implement AMP properly. This has come up frequently in the support forums. By sanitizing the entire document, this would no longer happen frequently.
2. The CSS in the Reader mode templates is output in a style.php template and the amp_post_template_css action. The style rules are not sent through the CSS sanitizer. This means that they are neither minified nor tree-shaken. If the entire template were sent through the sanitizers, then this would be a key benefit (and would resolve CSS in Reader mode is not minified #688).
3. By sending the entire reader mode templates through the sanitizers, this closer aligns with traditional theme templates and will facilitate transitioning from those classic post templates to standard theme templates (e.g. where $this->load_parts() is not used).
4. There are currently two entirely different code-paths for Reader vs non-Reader modes. There are opportunities to better harmonize the two, if we can use the code in AMP_Theme_Support for both.

---

This should make Reader mode templates available for Polylang to translate (topic).

By making Reader mode templates regular WordPress templates, we'll also be able to avoid situations where users copy Reader mode templates to be used in Transitional mode, only to find that they start getting fatal errors due to $this. (Examples: 1, 2, 3).

Update: This is probably a bit out of scope here. I think we should take the existing Reader mode post templates and port them to regular WordPress theme templates, but when we do so we need to introduce generic amp_head and amp_footer actions (and the like) which run instead of amp_post_template_head (à la wp_head) and amp_post_template_footer (à la wp_footer), similar to what we did for stories with amp_story_head. The amp_post_template_* actions should be deprecated because they are passed a AMP_Post_Template instance which is not relevant when templates can render any WordPress query, including index pages.

[kienstra]

Question about working on this

Hi @westonruter,
Hope you're having a great weekend.

You're the assignee here, so I wanted to ask if it's alright if I work on this.

Thanks, Weston!

[westonruter]

Yes, go ahead.

[kienstra]

Thanks!

[westonruter]

The solution here should also cause Polylang to end up working. Ref: https://wordpress.org/support/topic/static-home-page-with-polylang-2/page/2/#post-12066633

[schlessera]

This is on hold until we've processed #3662 and the linked document https://docs.google.com/document/d/1ROTa0QlCbW-wDxeZOkn8REEHaOLs22qSrBPKshcYcrE/edit?usp=sharing.

[westonruter]

@schlessera I took a stab at this, looking to see what is the minimal amount of changes which could be done while also getting all of the benefits: #3781.

[kienstra]

Steps To Test

1. Select 'Reader' mode
2. Create several posts or pages, with several blocks. Including embeds, images, and videos, etc...
3. Expected: The front-end of the AMP URLs shouldn't have obvious issues, and should be valid AMP per the AMP Chrome extension

(Functional testing won't really cover much of this work, as it involves sending the entire templates through the sanitizer. Testing that would require add_action() or add_filter() calls to test that any additional markup is sent through sanitizers.)

[csossi]

Verified in QA