Merge pull request #1143 from google/fix/1142-amp-dev-mode

Fix AMP dev mode support

Author: felixarntz

includes/Core/Admin_Bar/Admin_Bar.php

@@ -82,6 +82,9 @@ function( $wp_admin_bar ) {
 			99
 		);
 
+		// TODO: This can be removed at some point, see https://github.com/ampproject/amp-wp/pull/4001.
+		add_filter( 'amp_dev_mode_element_xpaths', array( $this, 'add_amp_dev_mode' ) );
+
 		$admin_bar_callback = function() {
 			if ( ! $this->is_active() ) {
 				return;
@@ -93,12 +96,9 @@ function( $wp_admin_bar ) {
 			// Enqueue styles.
 			$this->assets->enqueue_asset( 'googlesitekit_adminbar_css' );
 
-			if ( $this->context->is_amp() ) {
-				if ( ! $this->is_amp_dev_mode() ) {
-					// AMP Dev Mode support was added in v1.4, and if it is not enabled then short-circuit since scripts will be invalid.
-					return;
-				}
-				add_filter( 'amp_dev_mode_element_xpaths', array( $this, 'add_amp_dev_mode' ) );
+			if ( $this->context->is_amp() && ! $this->is_amp_dev_mode() ) {
+				// AMP Dev Mode support was added in v1.4, and if it is not enabled then short-circuit since scripts will be invalid.
+				return;
 			}
 
 			// Enqueue scripts.

includes/Core/Assets/Assets.php

@@ -288,7 +288,8 @@ private function add_amp_dev_mode_attributes( $assets ) {
 		add_filter(
 			'script_loader_tag',
 			function ( $tag, $handle ) use ( $assets ) {
-				if ( $this->context->is_amp() && isset( $assets[ $handle ] ) && $assets[ $handle ] instanceof Script ) {
+				// TODO: 'hoverintent-js' can be removed from here at some point, see https://github.com/ampproject/amp-wp/pull/3928.
+				if ( $this->context->is_amp() && ( isset( $assets[ $handle ] ) && $assets[ $handle ] instanceof Script || 'hoverintent-js' === $handle ) ) {
 					$tag = preg_replace( '/(?<=<script)(?=\s|>)/i', ' data-ampdevmode', $tag );
 				}
 				return $tag;
@@ -376,7 +377,7 @@ private function get_assets() {
 									'nonceEndpoint'   => admin_url( 'admin-ajax.php?action=rest-nonce' ),
 									'nonceMiddleware' => ( wp_installing() && ! is_multisite() ) ? '' : wp_create_nonce( 'wp_rest' ),
 									'rootURL'         => esc_url_raw( get_rest_url() ),
-								) 
+								)
 							),
 							'before'
 						);
@@ -719,6 +720,8 @@ private function add_async_defer_attribute( $tag, $handle ) {
 	 * @param array           $handles      List of handles to run before print callbacks for.
 	 */
 	private function run_before_print_callbacks( WP_Dependencies $dependencies, array $handles ) {
+		$is_amp = $this->context->is_amp();
+
 		foreach ( $handles as $handle ) {
 			if ( isset( $this->print_callbacks_done[ $handle ] ) ) {
 				continue;
@@ -728,11 +731,34 @@ private function run_before_print_callbacks( WP_Dependencies $dependencies, arra
 
 			if ( isset( $this->assets[ $handle ] ) ) {
 				$this->assets[ $handle ]->before_print();
+
+				// TODO: This can be removed at some point, see https://github.com/ampproject/amp-wp/pull/4001.
+				if ( $is_amp && $this->assets[ $handle ] instanceof Script ) {
+					$this->add_extra_script_amp_dev_mode( $handle );
+				}
 			}
 
 			if ( isset( $dependencies->registered[ $handle ] ) && is_array( $dependencies->registered[ $handle ]->deps ) ) {
 				$this->run_before_print_callbacks( $dependencies, $dependencies->registered[ $handle ]->deps );
 			}
 		}
 	}
+
+	/**
+	 * Adds a comment to all extra scripts so that they are considered compatible with AMP dev mode.
+	 *
+	 * {@see Assets::add_amp_dev_mode_attributes()} makes all registered scripts and stylesheets compatible, including
+	 * their potential inline additions. This method does the same for extra scripts, which are registered under the
+	 * 'data' key.
+	 *
+	 * @since n.e.x.t
+	 *
+	 * @param string $handle The handle of a registered script.
+	 */
+	private function add_extra_script_amp_dev_mode( $handle ) {
+		$data = wp_scripts()->get_data( $handle, 'data' ) ?: '';
+		if ( ! empty( $data ) && is_string( $data ) ) {
+			wp_scripts()->add_data( $handle, 'data', '/*googlesitekit*/ ' . $data );
+		}
+	}
 }