Add contextual awareness to Tag & Attribute sanitizer#3206
Add contextual awareness to Tag & Attribute sanitizer#3206schlessera wants to merge 7 commits intodevelopfrom
Conversation
|
As we currently don't have a single consistent iteration over the DOM for all sanitizers combined, we cannot easily share the contextual information across sanitizers. For solving something like a |
| } | ||
|
|
||
| // The type attribute of a template tag defines what templating engine to use. | ||
| switch ( $type->nodeValue ) { |
There was a problem hiding this comment.
This switch only has one case so far, but this properly communicates the intent that there are multiple possible values we need to consider here. Turning it into an if for the short-term would obfuscate this intent and probably lead to problems later on when extending.
| * used. | ||
| */ | ||
| protected function needs_contextual_nodes( DOMNode $node ) { | ||
| switch ( $node->nodeName ) { |
There was a problem hiding this comment.
This switch only has one case so far, but this properly communicates the intent that there are multiple possible values we need to consider here. Turning it into an if for the short-term would obfuscate this intent and probably lead to problems later on when extending.
…keep-attributes-with-mustache-placeholders-intact * 'develop' of github.com:ampproject/amp-wp: (113 commits) This converts the keyboard cut handler to equal a copy handler to avoid bugs Fix aria-label adding helper function Remove extra line I added in resolving merge conflict Fix alignment of arrow Fix custom CTA text for page attachment Fix cut handler by shortcut Cleanup of duplicated comment Add unit testing to `addVideoAriaLabel` Remove unused piece of code Remove Cloudflare AMP cache since deprecated Handle cut keyboard requests. (#3231) Page Attachment block (#3035) Keyboard & Right-Click Menu Copy + Paste (#3083) Animation Previews (#3104) Make internal methods inaccessible Omit the ecosystem link also when using a core theme Add skipped e2e test for the video block Add array_colum() pollyfill for PHP < 5.5 Add asserts to make sure we are not enqueueing both versions of dashicons Remove useless variable ...
There was a problem hiding this comment.
Please review my PR #3243 which builds on what you've done here to suggest the an alternative solution which I believe is simpler and more flexible (e.g. being able to be used also for checking if has_ancestor).
That PR targets this PR's branch, so once merged into this branch we can then wrap up this PR for 1.3. I updated the PR to point to develop.
|
Suggesting we close this PR in favor of #3243. |
This PR changes the way nodes are processed in the Tag & Attribute sanitizer from breadth-first to depth-first. This is mainly achieved by replacing the stack additions with an array_splice to replace the currently processed node.
It also adds a mechanism to track contextual information ("Are we currently within a template tag?") to make the processing aware of such context, and add the ability to conditionally act based on such context.
Change breakdown:
AMP_Contextclass as a container to track the current context.AMP_Contextual_Nodeclass as stack marker to track enter, toggle and leave operations for such context.Fixes #3137