[amp-autocomplete ] Fix invalid URL protocol for local development

[artneo-io]

Bug Description

Invalid URL protocol http: for attribute src for amp-autocomplete on local development.

The plugin triggers INVALID_URL_PROTOCOL for //localhost:3000/wp-json/neoblog/v1/search/ URL.

Expected Behaviour

It shouldn't be consider an error. To test amp-autocomplete I need to disable the plugin or select View with AMP disabled.

---

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

[westonruter]

Is your local development site running under HTTP and not HTTPS?

In looking at the AMP playground, you can see that http: is not valid AMP:

So on one hand, this would ideally be something that AMP core would update in the validator, to exempt localhost from having HTTPS enforced, similar to how Service Workers normally require HTTPS except for when on localhost, which is considered a secure origin.

Otherwise, to exempt http://localhost from being flagged as a src validation error, an alternative would be to force AMP Dev Mode for such elements when wp_get_environment_type() === 'local' (as of WP 5.5). In fact, amp_is_dev_mode() could be by default return true when on the local environment and the HTTP_HOST is localhost. That makes sense to me.

[westonruter]

The quick fix is to enable HTTPS for your local environment.

[artneo-io]

I develop locally using Browsersync – the HTTP version page refreshes faster than HTTPS.

[westonruter]

I suggest you conditionally add the data-ampdevmode attribute to this <amp-autocomplete> element when wp_get_environment_type() === 'local'.

And then also add this plugin code:

if ( wp_get_environment_type() === 'local' ) {
    add_filter( 'amp_dev_mode_enabled', '__return_true' );
}

[artneo-io]

Does adding add_filter( 'amp_dev_mode_enabled', '__return_true' ); already changes the plugin behavior somehow?

[westonruter]

It forces the html element to have the data-ampdevmode attribute. Normally this is only ever added if you are logged-in and the admin bar is displayed, since it's important that AMP Dev Mode documents not be accessible to crawlers, since they are explicitly not valid AMP. However, if your site is on a local environment then it's clearly not going to be accessible to crawlers, so it doesn't matter.

[westonruter]

See default value for this filter here:

amp-wp/includes/amp-helper-functions.php

Lines 1442 to 1462 in 27718b0

	/**
	* Filters whether AMP mode is enabled.
	*
	* When enabled, the data-ampdevmode attribute will be added to the document element and it will allow the
	* attributes to be added to the admin bar. It will also add the attribute to all elements which match the
	* queries for the expressions returned by the 'amp_dev_mode_element_xpaths' filter.
	*
	* @since 1.3
	* @param bool Whether AMP dev mode is enabled.
	*/
	return apply_filters(
	'amp_dev_mode_enabled',
	(
	// For the few sites that forcibly show the admin bar even when the user is logged out, only enable dev
	// mode if the user is actually logged in. This prevents the dev mode from being served to crawlers
	// when they index the AMP version. The theme support check disables dev mode in Reader mode.
	( is_admin_bar_showing() && is_user_logged_in() )
	||
	is_customize_preview()
	)
	);

[westonruter]

I think amending the logic as follows would also make sense:

--- a/includes/amp-helper-functions.php
+++ b/includes/amp-helper-functions.php
@@ -1458,6 +1458,16 @@ function amp_is_dev_mode() {
 			( is_admin_bar_showing() && is_user_logged_in() )
 			||
 			is_customize_preview()
+			||
+			(
+				! is_ssl()
+				&&
+				(
+					( function_exists( 'wp_get_environment_type' ) && 'local' === wp_get_environment_type() )
+					||
+					'localhost' === wp_parse_url( home_url(), PHP_URL_HOST )
+				)
+			)
 		)
 	);
 }

This can then be combined with extending the AMP_Dev_Mode_Sanitizer to automatically add data-ampdevmode attributes to all elements with src and action attributes that that have the http: protocol which point to the site. Nevertheless, it may be that some AMP components will just refuse to work if not on HTTPS as well, so your mileage may vary with this approach. If that is the case, a feature request to exempt localhost from the HTTPS requirement should be submitted to AMP core.

[westonruter]

This should also address an issue I've seen with wp-env, where when the PWA plugin is enabled, there are validation error due to the use of http: protocol for the Web App Manifest link. This is due to wp-env serving from http://localhost:8888

[pooja-muchandikar]

QA Passed ✅

Before with v2.4.1:

---

After the fix, this error is not visible. Verified with other plugins such as Web stories, PWA and WooCommerce nothing suspicious error observed it is working fine.