Simplify how validation errors get auto-accepted based on user prefer…

…ence
ampproject · Jun 7, 2018 · f864d67 · f864d67
1 parent 32a093e
commit f864d67
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 41 deletions.
diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -248,42 +248,19 @@ public static function get_validation_error_sanitization( $error ) {
 		return compact( 'status', 'forced' );
 	}
 
-	/**
-	 * Whether optionally-accepted validation errors are being suppressed.
-	 *
-	 * @var bool
-	 */
-	public static $suppress_optional_validation_error_acceptance = false;
-
 	/**
 	 * Automatically (forcibly) accept validation errors that arise.
 	 *
 	 * @since 1.0
 	 * @see AMP_Core_Theme_Sanitizer::get_acceptable_errors()
 	 *
 	 * @param array|true $acceptable_errors Acceptable validation errors, where keys are codes and values are either `true` or sparse array to check as subset. If just true, then all validation errors are accepted.
-	 * @param array      $args {
-	 *     Args.
-	 *
-	 *     @var bool $optional Whether the validation errors are accepted via option.
-	 * }
 	 */
-	public static function accept_validation_errors( $acceptable_errors, $args = array() ) {
+	public static function accept_validation_errors( $acceptable_errors ) {
 		if ( empty( $acceptable_errors ) ) {
 			return;
 		}
-
-		$args = array_merge(
-			array( 'optional' => false ),
-			$args
-		);
-
-		$optional = ! empty( $args['optional'] );
-		add_filter( 'amp_validation_error_sanitized', function( $sanitized, $error ) use ( $acceptable_errors, $optional ) {
-			if ( $optional && AMP_Validation_Error_Taxonomy::$suppress_optional_validation_error_acceptance ) {
-				return $sanitized;
-			}
-
+		add_filter( 'amp_validation_error_sanitized', function( $sanitized, $error ) use ( $acceptable_errors ) {
 			if ( true === $acceptable_errors ) {
 				return true;
 			}

diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php
@@ -159,23 +159,21 @@ public static function init( $args = array() ) {
 			}
 		} );
 
-		// Set sanitization options.
-		if ( amp_is_canonical() || AMP_Options_Manager::get_option( 'force_sanitization' ) ) {
-			AMP_Validation_Error_Taxonomy::accept_validation_errors( true, array( 'optional' => true ) );
-		} elseif ( AMP_Options_Manager::get_option( 'accept_tree_shaking' ) ) {
-			AMP_Validation_Error_Taxonomy::accept_validation_errors(
-				array(
-					AMP_Style_Sanitizer::TREE_SHAKING_ERROR_CODE => true,
-				),
-				array( 'optional' => true )
-			);
-		}
-
-		// @todo This is not great.
-		// Allow the validation errors to be modified in the admin.
-		AMP_Validation_Error_Taxonomy::$suppress_optional_validation_error_acceptance = true;
+		/*
+		 * Set sanitization options on the frontend. These filters get added only on the frontend so that
+		 * the user is able to keep track of the errors that they haven't seen before and decide whether
+		 * they need to get fixed (rejected) or not (accepted).
+		 */
 		add_action( 'template_redirect', function() {
-			AMP_Validation_Error_Taxonomy::$suppress_optional_validation_error_acceptance = false;
+			if ( amp_is_canonical() || AMP_Options_Manager::get_option( 'force_sanitization' ) ) {
+				AMP_Validation_Error_Taxonomy::accept_validation_errors( true );
+			} elseif ( AMP_Options_Manager::get_option( 'accept_tree_shaking' ) ) {
+				AMP_Validation_Error_Taxonomy::accept_validation_errors(
+					array(
+						AMP_Style_Sanitizer::TREE_SHAKING_ERROR_CODE => true,
+					)
+				);
+			}
 		} );
 
 		if ( self::$should_locate_sources ) {