-
Notifications
You must be signed in to change notification settings - Fork 385
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
Facilitate using AMP components outside of AMP documents (AMP in PWA, “dirty AMP”) #1013
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
51664cf
Facilitate using AMP components outside of AMP documents (dirty AMP)
westonruter 0154620
Move any AMP component scripts from body into head
westonruter 82e27ff
Improve registration and printing of AMP scripts
westonruter 6bedf70
Add missing version for amp-default style
westonruter File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,6 +119,12 @@ public static function finish_init() { | |
self::register_paired_hooks(); | ||
} | ||
|
||
// Enqueue AMP runtime. | ||
wp_enqueue_script( 'amp-runtime' ); | ||
|
||
// Enqueue default styles expected by sanitizer. | ||
wp_enqueue_style( 'amp-default', amp_get_asset_url( 'css/amp-default.css' ), array(), AMP__VERSION ); | ||
|
||
self::add_hooks(); | ||
self::$sanitizer_classes = amp_get_content_sanitizers(); | ||
self::$embed_handlers = self::register_content_embed_handlers(); | ||
|
@@ -212,9 +218,7 @@ public static function add_hooks() { | |
* in this case too we should defer to the theme as well to output the meta charset because it is possible the | ||
* install is not on utf-8 and we may need to do a encoding conversion. | ||
*/ | ||
add_action( 'wp_head', array( __CLASS__, 'add_amp_component_scripts' ), 10 ); | ||
add_action( 'wp_print_styles', array( __CLASS__, 'print_amp_styles' ), 0 ); // Print boilerplate before theme and plugin stylesheets. | ||
add_action( 'wp_enqueue_scripts', array( __CLASS__, 'enqueue_amp_default_styles' ), 9 ); | ||
add_action( 'wp_head', 'amp_add_generator_metadata', 20 ); | ||
add_action( 'wp_head', 'amp_print_schemaorg_metadata' ); | ||
|
||
|
@@ -566,16 +570,6 @@ public static function filter_paired_template_include( $template ) { | |
return $template; | ||
} | ||
|
||
/** | ||
* Print AMP script and placeholder for others. | ||
* | ||
* @link https://www.ampproject.org/docs/reference/spec#scrpt | ||
*/ | ||
public static function add_amp_component_scripts() { | ||
// Replaced after output buffering with all AMP component scripts. | ||
echo self::SCRIPTS_PLACEHOLDER; // phpcs:ignore WordPress.Security.EscapeOutput, WordPress.XSS.EscapeOutput | ||
} | ||
|
||
/** | ||
* Get canonical URL for current request. | ||
* | ||
|
@@ -748,69 +742,6 @@ public static function print_amp_styles() { | |
echo "<style amp-custom></style>\n"; // This will by populated by AMP_Style_Sanitizer. | ||
} | ||
|
||
/** | ||
* Adds default styles expected by sanitizer. | ||
*/ | ||
public static function enqueue_amp_default_styles() { | ||
wp_enqueue_style( 'amp-default', amp_get_asset_url( 'css/amp-default.css' ) ); | ||
} | ||
|
||
/** | ||
* Determine required AMP scripts. | ||
* | ||
* @param array $amp_scripts Initial scripts. | ||
* @return string Scripts to inject into the HEAD. | ||
*/ | ||
public static function get_amp_scripts( $amp_scripts ) { | ||
|
||
foreach ( self::$embed_handlers as $embed_handler ) { | ||
$amp_scripts = array_merge( | ||
$amp_scripts, | ||
$embed_handler->get_scripts() | ||
); | ||
} | ||
|
||
/** | ||
* List of components that are custom elements. | ||
* | ||
* Per the spec, "Most extensions are custom-elements." In fact, there is only one custom template. | ||
* | ||
* @link https://github.com/ampproject/amphtml/blob/cd685d4e62153557519553ffa2183aedf8c93d62/validator/validator.proto#L326-L328 | ||
* @link https://github.com/ampproject/amphtml/blob/cd685d4e62153557519553ffa2183aedf8c93d62/extensions/amp-mustache/validator-amp-mustache.protoascii#L27 | ||
*/ | ||
$custom_templates = array( 'amp-mustache' ); | ||
|
||
/** | ||
* Filters AMP component scripts before they are injected onto the output buffer for the response. | ||
* | ||
* Plugins may add their own component scripts which have been rendered but which the plugin doesn't yet | ||
* recognize. | ||
* | ||
* @since 0.7 | ||
* | ||
* @param array $amp_scripts AMP Component scripts, mapping component names to component source URLs. | ||
*/ | ||
$amp_scripts = apply_filters( 'amp_component_scripts', $amp_scripts ); | ||
|
||
$scripts = '<script async src="https://cdn.ampproject.org/v0.js"></script>'; // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript | ||
foreach ( $amp_scripts as $amp_script_component => $amp_script_source ) { | ||
|
||
$custom_type = 'custom-element'; | ||
if ( in_array( $amp_script_component, $custom_templates, true ) ) { | ||
$custom_type = 'custom-template'; | ||
} | ||
|
||
$scripts .= sprintf( | ||
'<script async %s="%s" src="%s"></script>', // phpcs:ignore WordPress.WP.EnqueuedResources, WordPress.XSS.EscapeOutput.OutputNotEscaped | ||
$custom_type, | ||
$amp_script_component, | ||
$amp_script_source | ||
); | ||
} | ||
|
||
return $scripts; | ||
} | ||
|
||
/** | ||
* Ensure markup required by AMP <https://www.ampproject.org/docs/reference/spec#required-markup>. | ||
* | ||
|
@@ -819,10 +750,18 @@ public static function get_amp_scripts( $amp_scripts ) { | |
* canonical URL by default if a singular post. | ||
* | ||
* @since 0.7 | ||
* @todo All of this might be better placed inside of a sanitizer. | ||
* | ||
* @param DOMDocument $dom Doc. | ||
*/ | ||
protected static function ensure_required_markup( DOMDocument $dom ) { | ||
$xpath = new DOMXPath( $dom ); | ||
|
||
// First ensure the mandatory amp attribute is present on the html element, as otherwise it will be stripped entirely. | ||
if ( ! $dom->documentElement->hasAttribute( 'amp' ) && ! $dom->documentElement->hasAttribute( '⚡️' ) ) { | ||
$dom->documentElement->setAttribute( 'amp', '' ); | ||
} | ||
|
||
$head = $dom->getElementsByTagName( 'head' )->item( 0 ); | ||
if ( ! $head ) { | ||
$head = $dom->createElement( 'head' ); | ||
|
@@ -872,6 +811,15 @@ protected static function ensure_required_markup( DOMDocument $dom ) { | |
) ); | ||
$head->appendChild( $rel_canonical ); | ||
} | ||
|
||
// Make sure scripts from the body get moved to the head. | ||
$scripts = array(); | ||
foreach ( $xpath->query( '//body//script[ @custom-element or @custom-template ]' ) as $script ) { | ||
$scripts[] = $script; | ||
} | ||
foreach ( $scripts as $script ) { | ||
$head->appendChild( $script ); | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -1008,15 +956,10 @@ public static function prepare_response( $response, $args = array() ) { | |
} | ||
$dom = AMP_DOM_Utils::get_dom( $response ); | ||
|
||
// First ensure the mandatory amp attribute is present on the html element, as otherwise it will be stripped entirely. | ||
if ( ! $dom->documentElement->hasAttribute( 'amp' ) && ! $dom->documentElement->hasAttribute( '⚡️' ) ) { | ||
$dom->documentElement->setAttribute( 'amp', '' ); | ||
} | ||
self::ensure_required_markup( $dom ); | ||
|
||
$assets = AMP_Content_Sanitizer::sanitize_document( $dom, self::$sanitizer_classes, $args ); | ||
|
||
self::ensure_required_markup( $dom ); | ||
|
||
// @todo If 'utf-8' is not the blog charset, then we'll need to do some character encoding conversation or "entityification". | ||
if ( 'utf-8' !== strtolower( get_bloginfo( 'charset' ) ) ) { | ||
/* translators: %s is the charset of the current site */ | ||
|
@@ -1032,13 +975,33 @@ public static function prepare_response( $response, $args = array() ) { | |
$response = "<!DOCTYPE html>\n"; | ||
$response .= AMP_DOM_Utils::get_content_from_dom_node( $dom, $dom->documentElement ); | ||
|
||
// Inject required scripts. | ||
$response = preg_replace( | ||
'#' . preg_quote( self::SCRIPTS_PLACEHOLDER, '#' ) . '#', | ||
self::get_amp_scripts( $assets['scripts'] ), | ||
$response, | ||
1 | ||
); | ||
$amp_scripts = $assets['scripts']; | ||
foreach ( self::$embed_handlers as $embed_handler ) { | ||
$amp_scripts = array_merge( | ||
$amp_scripts, | ||
$embed_handler->get_scripts() | ||
); | ||
} | ||
|
||
// Allow for embed handlers to override the default extension version by defining a different URL. | ||
foreach ( $amp_scripts as $handle => $value ) { | ||
if ( is_string( $value ) && wp_script_is( $handle, 'registered' ) ) { | ||
wp_scripts()->registered[ $handle ]->src = $value; | ||
} | ||
} | ||
|
||
// Print all scripts, some of which may have already been printed and inject into head. | ||
ob_start(); | ||
wp_print_scripts( array_keys( $amp_scripts ) ); | ||
$script_tags = ob_get_clean(); | ||
if ( ! empty( $script_tags ) ) { | ||
$response = preg_replace( | ||
'#(?=</head>)#', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the slash be escaped? |
||
$script_tags, | ||
$response, | ||
1 | ||
); | ||
} | ||
|
||
return $response; | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the slash be escaped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. This is doing a lookahead search to find
></script>
in the string to inject content before it.