Improve definition and filtering of sanitizer default args#1478
Closed
westonruter wants to merge 1 commit intodevelopfrom
Closed
Improve definition and filtering of sanitizer default args#1478westonruter wants to merge 1 commit intodevelopfrom
westonruter wants to merge 1 commit intodevelopfrom
Conversation
westonruter
commented
Sep 28, 2018
| 'AMP_Tag_And_Attribute_Sanitizer', // Always ultimate. | ||
| ); | ||
|
|
||
| // @todo This doesn't help for classes that get added via amp_content_sanitizers. Does that matter? |
Member
Author
There was a problem hiding this comment.
In practice this probably doesn't matter. If someone is adding a new sanitizer, they'd be doing so via a filter anyway. They would just be required to supply the defaults when they do so. For example:
add_filter( 'amp_content_sanitizers', function( $sanitizers ) {
require_once __DIR__ . '/class-sanitizer.php'
return array_merge(
$sanitizers,
array(
'\Foo\Sanitizer' => Sanitizer::get_default_args(),
)
);
} );
This was referenced Oct 5, 2018
Member
Author
|
This should be considered as part of the whole refactor. |
17 tasks
300352c to
623aa91
Compare
Move amp_get_content_sanitizers defaults to get_default_args
623aa91 to
3036369
Compare
Member
Author
|
Closing this as stale, and the approach we take should probably be part of the modularization effort. See #4570. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
At the moment the default args for the sanitizers are not mode available for plugins to filter. This means, for example, if a plugin wants to augment default args they have to currently copy all of them:
This is not ideal. This PR allows you to instead do:
The
AMP_Base_Sanitizerclass gets a newget_default_args()static method which is called up front when obtaining the list of sanitizers and their args. This then exposes the default args to theamp_content_sanitizersfilter to customize.This PR also deprecates the
DEFAULT_ARGSclass variable and consolidates the location of default args to be all defined in thisget_default_args()methods.