-
Notifications
You must be signed in to change notification settings - Fork 384
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
Refactor storage of validation errors to use taxonomy terms #1093
Changes from all commits
2d5a341
1105fe3
36cf381
044d3c2
95487df
212171f
65999ea
c482b37
a6d2f70
546c163
52f4cdd
8f31472
1e1516d
1cb4a69
aadadaf
f885262
7af2db5
03bf406
c5e4202
3916d2e
fd22331
5341f2b
efc6661
8348190
85989eb
159d808
7048994
3c0d724
99759b0
de5c323
c9a33b1
d30ab72
13c14a9
71d36e7
4cf66ba
9dde322
9a79f05
932ec66
8a92355
c58f897
e00e16d
6cd5e7e
9d159d5
7869b63
1f315a8
85a0585
7c0a02d
90eff4f
69055c0
f17df99
f960f62
439bca5
f29eaa2
c3db57e
2b1985a
a4312b4
cfa8bf5
543b935
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,9 +63,6 @@ public static function send_header( $name, $value, $args = array() ) { | |
* Send Server-Timing header. | ||
* | ||
* @since 1.0 | ||
* @todo What is the ordering in Chrome dev tools? What are the colors about? | ||
* @todo Is there a better name standardization? | ||
* @todo Is there a way to indicate nested server timings, so an outer method's own time can be seen separately from the inner method's time? | ||
* | ||
* @param string $name Name. | ||
* @param float $duration Duration. If negative, will be added to microtime( true ). Optional. | ||
|
@@ -75,7 +72,7 @@ public static function send_header( $name, $value, $args = array() ) { | |
public static function send_server_timing( $name, $duration = null, $description = null ) { | ||
$value = $name; | ||
if ( isset( $description ) ) { | ||
$value .= sprintf( ';desc=%s', wp_json_encode( $description ) ); | ||
$value .= sprintf( ';desc="%s"', str_replace( array( '\\', '"' ), '', substr( $description, 0, 100 ) ) ); | ||
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. Replacing 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. This logic here is specifically for ensuring the well-formedness of the |
||
} | ||
if ( isset( $duration ) ) { | ||
if ( $duration < 0 ) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,8 +100,11 @@ public static function init() { | |
return; | ||
} | ||
|
||
AMP_Validation_Manager::init( array( | ||
'should_locate_sources' => AMP_Validation_Manager::should_validate_response(), | ||
) ); | ||
|
||
self::$init_start_time = microtime( true ); | ||
AMP_Validation_Utils::init(); | ||
|
||
self::purge_amp_query_vars(); | ||
self::handle_xhr_request(); | ||
|
@@ -154,15 +157,20 @@ public static function finish_init() { | |
|
||
self::add_hooks(); | ||
self::$sanitizer_classes = amp_get_content_sanitizers(); | ||
self::$sanitizer_classes = AMP_Validation_Manager::filter_sanitizer_args( self::$sanitizer_classes ); | ||
self::$embed_handlers = self::register_content_embed_handlers(); | ||
} | ||
|
||
/** | ||
* Redirect to canonical URL if the AMP URL was loaded, since canonical is now AMP. | ||
* | ||
* @since 0.7 | ||
* @since 1.0 Added $exit param. | ||
* @todo Rename to redirect_non_amp(). | ||
* | ||
* @param bool $exit Whether to exit after redirecting. | ||
*/ | ||
public static function redirect_canonical_amp() { | ||
public static function redirect_canonical_amp( $exit = true ) { | ||
if ( false !== get_query_var( amp_get_slug(), false ) ) { // Because is_amp_endpoint() now returns true if amp_is_canonical(). | ||
$url = preg_replace( '#^(https?://.+?)(/.*)$#', '$1', home_url( '/' ) ); | ||
if ( isset( $_SERVER['REQUEST_URI'] ) ) { | ||
|
@@ -171,8 +179,14 @@ public static function redirect_canonical_amp() { | |
|
||
$url = amp_remove_endpoint( $url ); | ||
|
||
wp_safe_redirect( $url, 302 ); // Temporary redirect because canonical may change in future. | ||
exit; | ||
/* | ||
* Temporary redirect because AMP URL may return when blocking validation errors | ||
* occur or when a non-canonical AMP theme is used. | ||
*/ | ||
wp_safe_redirect( $url, 302 ); | ||
if ( $exit ) { | ||
exit; | ||
} | ||
} | ||
} | ||
|
||
|
@@ -191,7 +205,13 @@ public static function is_paired_available() { | |
return false; | ||
} | ||
|
||
if ( is_singular() && ! post_supports_amp( get_queried_object() ) ) { | ||
/** | ||
* Queried object. | ||
* | ||
* @var WP_Post $queried_object | ||
*/ | ||
$queried_object = get_queried_object(); | ||
if ( is_singular() && ! post_supports_amp( $queried_object ) ) { | ||
return false; | ||
} | ||
|
||
|
@@ -275,11 +295,6 @@ public static function add_hooks() { | |
$priority = defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX; // phpcs:ignore PHPCompatibility.PHP.NewConstants.php_int_minFound | ||
add_action( 'template_redirect', array( __CLASS__, 'start_output_buffering' ), $priority ); | ||
|
||
// Add validation hooks *after* output buffering has started for the response. | ||
if ( AMP_Validation_Utils::should_validate_response() ) { | ||
AMP_Validation_Utils::add_validation_hooks(); | ||
} | ||
|
||
// Commenting hooks. | ||
add_filter( 'wp_list_comments_args', array( __CLASS__, 'set_comments_walker' ), PHP_INT_MAX ); | ||
add_filter( 'comment_form_defaults', array( __CLASS__, 'filter_comment_form_defaults' ) ); | ||
|
@@ -408,7 +423,7 @@ protected static function wp_kses_amp_mustache( $text ) { | |
* | ||
* @param string $url Comment permalink to redirect to. | ||
* @param WP_Comment $comment Posted comment. | ||
* @return string URL. | ||
* @return string|null URL if redirect to be done; otherwise function will exist. | ||
*/ | ||
public static function filter_comment_post_redirect( $url, $comment ) { | ||
$theme_support = get_theme_support( 'amp' ); | ||
|
@@ -443,6 +458,7 @@ public static function filter_comment_post_redirect( $url, $comment ) { | |
wp_send_json( array( | ||
'message' => self::wp_kses_amp_mustache( $message ), | ||
) ); | ||
return null; | ||
} | ||
|
||
/** | ||
|
@@ -631,7 +647,7 @@ public static function amend_comment_form() { | |
* @see get_query_template() | ||
* | ||
* @param array $templates Template hierarchy. | ||
* @returns array Templates. | ||
* @return array Templates. | ||
*/ | ||
public static function filter_paired_template_hierarchy( $templates ) { | ||
$support = get_theme_support( 'amp' ); | ||
|
@@ -848,6 +864,13 @@ public static function print_amp_styles() { | |
* @param DOMDocument $dom Doc. | ||
*/ | ||
public static function ensure_required_markup( DOMDocument $dom ) { | ||
/** | ||
* Elements. | ||
* | ||
* @var DOMElement $meta | ||
* @var DOMElement $script | ||
* @var DOMElement $link | ||
*/ | ||
$head = $dom->getElementsByTagName( 'head' )->item( 0 ); | ||
if ( ! $head ) { | ||
$head = $dom->createElement( 'head' ); | ||
|
@@ -856,11 +879,6 @@ public static function ensure_required_markup( DOMDocument $dom ) { | |
$meta_charset = null; | ||
$meta_viewport = null; | ||
foreach ( $head->getElementsByTagName( 'meta' ) as $meta ) { | ||
/** | ||
* Meta. | ||
* | ||
* @var DOMElement $meta | ||
*/ | ||
if ( $meta->hasAttribute( 'charset' ) && 'utf-8' === strtolower( $meta->getAttribute( 'charset' ) ) ) { // @todo Also look for meta[http-equiv="Content-Type"]? | ||
$meta_charset = $meta; | ||
} elseif ( 'viewport' === $meta->getAttribute( 'name' ) ) { | ||
|
@@ -1012,17 +1030,18 @@ public static function filter_customize_partial_render( $partial ) { | |
* @since 0.7 | ||
* | ||
* @param string $response HTML document response. By default it expects a complete document. | ||
* @param array $args { | ||
* Args to send to the preprocessor/sanitizer. | ||
* | ||
* @type callable $remove_invalid_callback Function to call whenever a node is removed due to being invalid. | ||
* } | ||
* @param array $args Args to send to the preprocessor/sanitizer. | ||
* @return string AMP document response. | ||
* @global int $content_width | ||
*/ | ||
public static function prepare_response( $response, $args = array() ) { | ||
global $content_width; | ||
|
||
if ( isset( $args['validation_error_callback'] ) ) { | ||
_doing_it_wrong( __METHOD__, 'Do not supply validation_error_callback arg.', '1.0' ); | ||
unset( $args['validation_error_callback'] ); | ||
} | ||
|
||
/* | ||
* Check if the response starts with HTML markup. | ||
* Without this check, JSON responses will be erroneously corrupted, | ||
|
@@ -1032,25 +1051,23 @@ public static function prepare_response( $response, $args = array() ) { | |
return $response; | ||
} | ||
|
||
$is_validation_debug_mode = isset( $_REQUEST[ AMP_Validation_Utils::DEBUG_QUERY_VAR ] ); // WPCS: csrf ok. | ||
|
||
$args = array_merge( | ||
array( | ||
'content_max_width' => ! empty( $content_width ) ? $content_width : AMP_Post_Template::CONTENT_MAX_WIDTH, // Back-compat. | ||
'use_document_element' => true, | ||
'allow_dirty_styles' => self::is_customize_preview_iframe(), // Dirty styles only needed when editing (e.g. for edit shortcodes). | ||
'allow_dirty_scripts' => is_customize_preview(), // Scripts are always needed to inject changeset UUID. | ||
'disable_invalid_removal' => $is_validation_debug_mode, | ||
'enable_response_caching' => ( | ||
( ! defined( 'WP_DEBUG' ) || true !== WP_DEBUG ) | ||
&& | ||
! AMP_Validation_Utils::should_validate_response() | ||
! AMP_Validation_Manager::should_validate_response() | ||
), | ||
), | ||
$args | ||
); | ||
|
||
// Return cache if enabled and found. | ||
$response_cache_key = null; | ||
if ( true === $args['enable_response_caching'] ) { | ||
// Set response cache hash, the data values dictates whether a new hash key should be generated or not. | ||
$response_cache_key = md5( wp_json_encode( array( | ||
|
@@ -1110,15 +1127,34 @@ public static function prepare_response( $response, $args = array() ) { | |
$dom_serialize_start = microtime( true ); | ||
self::ensure_required_markup( $dom ); | ||
|
||
if ( ! AMP_Validation_Manager::should_validate_response() && AMP_Validation_Manager::has_blocking_validation_errors() ) { | ||
if ( amp_is_canonical() ) { | ||
$dom->documentElement->removeAttribute( 'amp' ); | ||
|
||
/* | ||
* Make sure that document.write() is disabled to prevent dynamically-added content (such as added | ||
* via amp-live-list) from wiping out the page by introducing any scripts that call this function. | ||
*/ | ||
if ( $head ) { | ||
$script = $dom->createElement( 'script' ); | ||
$script->appendChild( $dom->createTextNode( 'document.addEventListener( "DOMContentLoaded", function() { document.write = function( text ) { throw new Error( "[AMP-WP] Prevented document.write() call with: " + text ); }; } );' ) ); | ||
$head->appendChild( $script ); | ||
} | ||
} else { | ||
self::redirect_canonical_amp( false ); | ||
return esc_html__( 'Redirecting to non-AMP version.', 'amp' ); | ||
} | ||
} | ||
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. This conditional is the critical piece to the whole thing. |
||
|
||
// @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 */ | ||
trigger_error( esc_html( sprintf( __( 'The database has the %s encoding when it needs to be utf-8 to work with AMP.', 'amp' ), get_bloginfo( 'charset' ) ) ), E_USER_WARNING ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error | ||
} | ||
|
||
if ( AMP_Validation_Utils::should_validate_response() ) { | ||
AMP_Validation_Utils::finalize_validation( $dom, array( | ||
'remove_source_comments' => ! $is_validation_debug_mode, | ||
if ( AMP_Validation_Manager::should_validate_response() ) { | ||
AMP_Validation_Manager::finalize_validation( $dom, array( | ||
'remove_source_comments' => ! isset( $_GET['amp_preserve_source_comments'] ), // WPCS: CSRF. | ||
) ); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,7 @@ public static function register_settings() { | |
); | ||
|
||
add_action( 'update_option_' . self::OPTION_NAME, array( __CLASS__, 'maybe_flush_rewrite_rules' ), 10, 2 ); | ||
add_action( 'admin_notices', array( __CLASS__, 'persistent_object_caching_notice' ) ); | ||
} | ||
|
||
/** | ||
|
@@ -259,4 +260,20 @@ public static function update_analytics_options( $data ) { | |
_deprecated_function( __METHOD__, '0.6', __CLASS__ . '::update_option' ); | ||
return self::update_option( 'analytics', wp_unslash( $data ) ); | ||
} | ||
|
||
/** | ||
* Outputs an admin notice if persistent object cache is not present. | ||
* | ||
* @return void | ||
*/ | ||
public static function persistent_object_caching_notice() { | ||
if ( ! wp_using_ext_object_cache() && 'toplevel_page_' . self::OPTION_NAME === get_current_screen()->id ) { | ||
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.
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. I tried swapping that out and it worked but it broke the unit test. Feel free to amend the PR with the change. |
||
printf( | ||
'<div class="notice notice-warning"><p>%s <a href="%s">%s</a></p></div>', | ||
esc_html__( 'The AMP plugin performs at its best when persistent object cache is enabled.', 'amp' ), | ||
esc_url( 'https://codex.wordpress.org/Class_Reference/WP_Object_Cache#Persistent_Caching' ), | ||
esc_html__( 'More details', 'amp' ) | ||
); | ||
} | ||
} | ||
} |
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.
It's nice how this is now organized in the
includes/validation/
directory.