Skip to content

Commit

Permalink
Ensure auto-accepting sanitization is always performed for stories; s…
Browse files Browse the repository at this point in the history 
…implify warning notice
  • Loading branch information
@westonruter
westonruter committed Jun 11, 2019
1 parent bbc6f0f commit 9737cd7
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 49 deletions.
91 changes: 47 additions & 44 deletions assets/src/block-validation/helpers/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ export const updateValidationErrors = () => {
export const maybeDisplayNotice = () => {
const { getValidationErrors, isSanitizationAutoAccepted, getReviewLink } = select( 'amp/block-validation' );
const { createWarningNotice } = dispatch( 'core/notices' );
const { getCurrentPost } = select( 'core/editor' );

const validationErrors = getValidationErrors();
const validationErrorCount = validationErrors.length;
Expand All @@ -174,54 +175,56 @@ export const maybeDisplayNotice = () => {
const blockValidationErrors = validationErrors.filter( ( { clientId } ) => clientId );
const blockValidationErrorCount = blockValidationErrors.length;

if ( blockValidationErrorCount > 0 ) {
noticeMessage += ' ' + sprintf(
/* translators: %s: number of block errors. */
_n(
'%s issue is directly due to content here.',
'%s issues are directly due to content here.',
blockValidationErrorCount,
'amp'
),
blockValidationErrorCount
);
} else if ( validationErrors.length === 1 ) {
noticeMessage += ' ' + __( 'The issue is not directly due to content here.', 'amp' );
} else {
noticeMessage += ' ' + __( 'The issues are not directly due to content here.', 'amp' );
}

noticeMessage += ' ';

if ( isSanitizationAutoAccepted() ) {
const rejectedBlockValidationErrors = blockValidationErrors.filter( ( error ) => {
return (
VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status ||
VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status
);
} );

const rejectedValidationErrors = validationErrors.filter( ( error ) => {
return (
VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status ||
VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status
if ( 'amp_story' !== getCurrentPost().type ) {
if ( blockValidationErrorCount > 0 ) {
noticeMessage += ' ' + sprintf(
/* translators: %s: number of block errors. */
_n(
'%s issue is directly due to content here.',
'%s issues are directly due to content here.',
blockValidationErrorCount,
'amp'
),
blockValidationErrorCount
);
} );

const totalRejectedErrorsCount = rejectedBlockValidationErrors.length + rejectedValidationErrors.length;
} else if ( validationErrors.length === 1 ) {
noticeMessage += ' ' + __( 'The issue is not directly due to content here.', 'amp' );
} else {
noticeMessage += ' ' + __( 'The issues are not directly due to content here.', 'amp' );
}

if ( totalRejectedErrorsCount === 0 ) {
noticeMessage += __( 'However, your site is configured to automatically accept sanitization of the offending markup.', 'amp' );
noticeMessage += ' ';

if ( isSanitizationAutoAccepted() ) {
const rejectedBlockValidationErrors = blockValidationErrors.filter( ( error ) => {
return (
VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status ||
VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status
);
} );

const rejectedValidationErrors = validationErrors.filter( ( error ) => {
return (
VALIDATION_ERROR_NEW_REJECTED_STATUS === error.status ||
VALIDATION_ERROR_ACK_REJECTED_STATUS === error.status
);
} );

const totalRejectedErrorsCount = rejectedBlockValidationErrors.length + rejectedValidationErrors.length;

if ( totalRejectedErrorsCount === 0 ) {
noticeMessage += __( 'However, your site is configured to automatically accept sanitization of the offending markup.', 'amp' );
} else {
noticeMessage += _n(
'Your site is configured to automatically accept sanitization errors, but this error could be from when auto-acceptance was not selected, or from manually rejecting an error.',
'Your site is configured to automatically accept sanitization errors, but these errors could be from when auto-acceptance was not selected, or from manually rejecting an error.',
validationErrors.length,
'amp'
);
}
} else {
noticeMessage += _n(
'Your site is configured to automatically accept sanitization errors, but this error could be from when auto-acceptance was not selected, or from manually rejecting an error.',
'Your site is configured to automatically accept sanitization errors, but these errors could be from when auto-acceptance was not selected, or from manually rejecting an error.',
validationErrors.length,
'amp'
);
noticeMessage += __( 'Non-accepted validation errors prevent AMP from being served, and the user will be redirected to the non-AMP version.', 'amp' );
}
} else {
noticeMessage += __( 'Non-accepted validation errors prevent AMP from being served, and the user will be redirected to the non-AMP version.', 'amp' );
}

const options = {
Expand Down
6 changes: 3 additions & 3 deletions includes/class-amp-theme-support.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ public static function read_theme_support() {
self::$support_added_via_theme = $is_paired ? self::TRANSITIONAL_MODE_SLUG : self::STANDARD_MODE_SLUG;

/*
* If the theme has transitional support, allow the user to opt for AMP first mode via an option, since a theme
* If the theme has transitional support, allow the user to opt for AMP-first mode via an option, since a theme
* in transitional mode entails that it supports serving templates as both AMP and non-AMP, and this it is
* able to serve AMP-first pages just as well as paired pages. Otherwise, consider that the the mode was
* not set at all via option.
Expand Down Expand Up @@ -411,7 +411,7 @@ public static function ensure_proper_amp_location( $exit = true ) {

if ( amp_is_canonical() || is_singular( AMP_Story_Post_Type::POST_TYPE_SLUG ) ) {
/*
* When AMP first/canonical, then when there is an /amp/ endpoint or ?amp URL param,
* When AMP-first/canonical, then when there is an /amp/ endpoint or ?amp URL param,
* then a redirect needs to be done to the URL without any AMP indicator in the URL.
* Permanent redirect is used for unauthenticated users since switching between modes
* should happen infrequently. For admin users, this is kept temporary to allow them
Expand Down Expand Up @@ -2119,7 +2119,7 @@ public static function prepare_response( $response, $args = array() ) {

if ( $blocking_error_count > 0 && ! AMP_Validation_Manager::should_validate_response() ) {
/*
* In AMP first, strip html@amp attribute to prevent GSC from complaining about a validation error
* In AMP-first, strip html@amp attribute to prevent GSC from complaining about a validation error
* already surfaced inside of WordPress. This is intended to not serve dirty AMP, but rather a
* non-AMP document (intentionally not valid AMP) that contains the AMP runtime and AMP components.
*/
Expand Down
10 changes: 9 additions & 1 deletion includes/validation/class-amp-validated-url-post-type.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,14 @@ public static function store_validation_errors( $validation_errors, $url, $args
);
}

$is_story = (
isset( $args['queried_object'], $args['queried_object']['type'], $args['queried_object']['id'] )
&&
'post' === $args['queried_object']['type']
&&
AMP_Story_Post_Type::POST_TYPE_SLUG === get_post_type( $args['queried_object']['id'] )
);

/*
* The details for individual validation errors is stored in the amp_validation_error taxonomy terms.
* The post content just contains the slugs for these terms and the sources for the given instance of
Expand Down Expand Up @@ -714,7 +722,7 @@ public static function store_validation_errors( $validation_errors, $url, $args
'term_group' => $sanitization['status'],
)
);
} elseif ( AMP_Validation_Manager::is_sanitization_auto_accepted() ) {
} elseif ( AMP_Validation_Manager::is_sanitization_auto_accepted() || $is_story ) {
$term_data['term_group'] = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS;
wp_update_term(
$term_id,
Expand Down
2 changes: 1 addition & 1 deletion includes/validation/class-amp-validation-manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2029,7 +2029,7 @@ public static function enqueue_block_validation() {
$enabled_status = $status_and_errors['status'];

$data = array(
'isSanitizationAutoAccepted' => self::is_sanitization_auto_accepted(),
'isSanitizationAutoAccepted' => self::is_sanitization_auto_accepted() || AMP_Story_Post_Type::POST_TYPE_SLUG === get_post_type(),
'possibleStatuses' => array( AMP_Post_Meta_Box::ENABLED_STATUS, AMP_Post_Meta_Box::DISABLED_STATUS ),
'defaultStatus' => $enabled_status,
);
Expand Down

0 comments on commit 9737cd7

Please sign in to comment.