Skip to content
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 validation errors #4568

Closed
schlessera opened this issue Apr 11, 2020 · 2 comments
Closed

Refactor validation errors #4568

schlessera opened this issue Apr 11, 2020 · 2 comments
Assignees
Labels
Compatibility Tool Developer Tools for AMP Debugging Epic Groomed P2 Low priority Sanitizers WS:Perf Work stream for Metrics, Performance and Optimizer

Comments

@schlessera
Copy link
Collaborator

The creation, storage, communication and presentation of validation errors has organically grown with the scope of the plugin.

Right now, validation errors are passed around as arbitrarily structured arrays that contain multiple values, and the representation needs to be manually kept in sync with the different types of errors to be able to make sense of the data structure.

The entire system around validation errors needs to be redesigned to provide a sane public interface that we can provide in the future generic PHP libraries for the sanitizer and the validator.

A good first step is the error handling in the optimizer library. This should be extracted into either the ampproject/common library or a separate library and expanded to cover all phases of AMP usage.

@schlessera schlessera added Compatibility Tool Developer Tools for AMP Debugging Sanitizers labels Apr 11, 2020
@schlessera schlessera self-assigned this Apr 11, 2020
@kmyram kmyram added the WS:Perf Work stream for Metrics, Performance and Optimizer label Aug 5, 2020
@westonruter
Copy link
Member

Related: #3875

@westonruter
Copy link
Member

Example of how validation error is manipulated by an array via the amp_validation_error filter in #4753:

add_filter( 'amp_validation_error', function ( $error ) {
	if (
		isset( $error['code'], $error['node_name'], $error['text'] )
		&&
		'DISALLOWED_TAG' === $error['code']
		&&
		'script' === $error['node_name']
	) {
		$error['text'] = preg_replace( '/"[0-9a-f]{10}"/', '__HEXSTR10__', $error['text'] );
	}
	return $error;
} );

@kmyram kmyram added Groomed P2 Low priority labels Mar 2, 2021
@westonruter westonruter closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compatibility Tool Developer Tools for AMP Debugging Epic Groomed P2 Low priority Sanitizers WS:Perf Work stream for Metrics, Performance and Optimizer
Projects
Archived in project
Development

No branches or pull requests

3 participants