Facilitate documents in AMP Dev Mode to be unmarked as AMP pages when site is in Standard mode

[westonruter]

Feature description

This originally was in discussed in Automattic/jetpack#16901 (review), where AMP Dev Mode was being explored as a way to serve pages managed by the AMP runtime which also include invalid AMP markup which is marked using data-ampdevmode. I wrote Automattic/jetpack#16901 (review):

There's a key difficulty with attempting to use AMP Dev Mode for exempting certain elements from AMP validation. And that is the AMP pages in Dev Mode will always have one validation error:

Tag 'html' marked with attribute 'data-ampdevmode'. Validator will suppress errors regarding any other tag with this attribute.

When logged-in to WordPress and the admin bar is showing, Dev Mode is enabled in order to include the additional scripts on the page and intentionally not be a valid AMP page. Nevertheless, these AMP-invalid scripts are only present when the user is logged in: they won't show up when being crawled by Googlebot. If, however, you force AMP Dev Mode to be enabled then users will start seeing Google Search Console warnings for AMP validation errors.

If intending to use AMP Dev Mode in this way, we'd need to also provide the capability to omit the amp attribute from the html element. In this way, the page won't even be marked as AMP and so GSC won't try to validated it as an AMP page. This capability would make sense for a site in Standard mode.

For example, let's say that you force Dev Mode to be enabled via:

add_filter( 'amp_dev_mode_enabled', '__return_true' );

And then we introduce a new filter for whether the document should be marked as AMP, so you could turn it off:

add_filter( 'amp_dev_mode_document_marked', '__return_false' ); // Tentative naming.

This really should only be allowed on a Standard mode site, as if you do it on a paired mode site, crawlers will navigate to the amphtml links and complain about being linked to a non-AMP page.

Then on a Standard mode site, you'd have an HTML page with the AMP runtime but it would not be marked as AMP. Note that this usage of AMP is not yet official supported. It will only be really safe/supported to do this once Bento AMP ships, as otherwise the AMP runtime is not expecting for 3rd-party JS to be mutating the document.

Note that omitting the amp attribute from the html element is also the behavior of Standard mode sites that have excessive CSS: instead of stripping out the excessive CSS to make it a valid AMP page (which is the behavior in Transitional mode), in Standard mode the default behavior is to not strip it out and then to just mark the page as non-AMP. This is because in Standard mode there is no non-AMP version to go to if the AMP version is broken due to the removed styles.

Come to think of it, if Dev Mode is enabled and no user is logged in, the behavior could be to just remove the the amp attribute by default. We wouldn't even need to introduce a filter. Users who are logged-in would still get the AMP Validator informing them of any validation errors that are leaking out of the sanitizers, but visitors who are not logged-in (e.g. Googlebot) would not see it as an AMP page.

So this condition here:

amp-wp/includes/class-amp-theme-support.php

Lines 2059 to 2064 in e99ff09

	// Ensure the mandatory amp attribute is present on the html element.
	if ( ! $dom->documentElement->hasAttribute( Attribute::AMP )
	&& ! $dom->documentElement->hasAttribute( Attribute::AMP_EMOJI )
	&& ! $dom->documentElement->hasAttribute( Attribute::AMP_EMOJI_ALT ) ) {
	$dom->documentElement->setAttribute( Attribute::AMP, '' );
	}

Would take into account whether $dom->documentElement->hasAttribute( 'data-ampdevmode ). If not, those AMP attributes should all be removed rather than being added, for unauthenticated requests. This should probably only apply to to sites in Standard mode.

Also discussed in Automattic/newspack-plugin#688.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

[coolrecep]

I think I have a problem related to this. Google does not accept our pages as AMP compatible and gives error:

Tag 'html' marked with attribute 'data-ampdevmode'. Validator will suppress errors regarding any other tag with this attribute.

[westonruter]

@coolov That's right. That's why this issue proposed a filter like amp_dev_mode_document_marked (name tentative) which would allow you to omit the amp attribute from such pages. When that attribute is absent, then the AMP page will not be checked for AMP validity. This would only make sense on a Standard mode site. If it were tried on a site in Transitional mode or Reader mode (which are both Paired AMP), then Google would complain about the link[rel=amphtml] not pointing to an AMP page.

[westonruter]

The better way forward here is what we've been discussing lately in terms of sandbox levels:

1. [Least restrictive] Convert markup to Bento, but leave other markup intact. Corresponds to "dirty AMP".
2. [Middle-ground] Convert markup to Bento, do CSS processing, and remove any non-Bento markup. Depends on custom scripts being packaged in Bento components.
3. [Most restrictive, same as today] Convert markup to Bento, do CSS processing, and remove any non-Bento markup that is not also AMP. Add amp attribute to document.

[westonruter]

Also, the levels should also indicate what is the least restrictive sandboxing enforcement. Just because I select L1 this doesn't mean that I will necessarily be invalid AMP. If there is nothing on the page that is invalid AMP, then the page itself would be L3 but other pages on the site may be L1. In the same way for L2: you may have a site that uses uncertified components, but on some pages only certified components are used. For the latter pages, they would be L3.

[bartoszgadomski]

QA passed

I tested what has been implemented in #6518, but the amp_using_native_img filter name provided in the PR description and code has been replaced with amp_native_img_used in #6527 (see: #6527 (comment)).

Default	With amp_native_img_used set to true

Validation errors has not been raised for <img> element.

Plugin used during testing:

<?php
/**
 * Plugin name: Test native img
 */
add_filter( 'amp_native_img_used', '__return_true' );