-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
WP_HTML_Tag_Processor: Inject dynamic data to block HTML markup in PHP #42485
Merged
Merged
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
4887c5c
Introduce `WP_HTML_Walker` for reliably modifying HTML attributes.
dmsnell 3f148db
Add TODO for `/` in attribute area
dmsnell 6d089f2
Add TODO for handling character references in `class` attribute
dmsnell 2dbfa03
Add TODO for attribute escaping
dmsnell fadd51b
Ignore slashes when parsing attribute names
adamziel 17879e8
Remove test_get_tag_returns_raw_open_tag_name as get_tag always retur…
adamziel bdd56a6
Correctly check the indices of <!-- characters in skip_script_data -
adamziel 8a48fab
Fix linting issues
gziolo e9a71d6
Improve existing unit tests
gziolo 9bcfd3a
Don't close the walker in the __toString() method
adamziel f6d1bcc
Code style update
adamziel 541d86b
Update phpunit/html/wp-html-walker-test.php
adamziel cdd8ae0
Update phpunit/html/wp-html-walker-test.php
adamziel 1852cf7
Update lib/experimental/html/class-wp-html-walker.php
adamziel 6c4647d
Update lib/experimental/html/class-wp-html-walker.php
adamziel 4554dd3
Update lib/experimental/html/class-wp-html-walker.php
dmsnell e2eb423
Refactor early-abort to remove level of nesting
dmsnell 68b55c4
Rewrite code to separate stages of computation
dmsnell f664c08
Expand name of $c to $character
dmsnell 147647b
typo: type of integer is `int`
dmsnell d8b8ddd
typo: type of integer is `int`
dmsnell a325fa3
Reorder the test methods
adamziel a4fb703
Add the missing error messages
adamziel c017420
Reorder tests to keep set_attribute tests grouped together
adamziel 07041ee
Update phpunit/html/wp-html-walker-test.php
adamziel 63f4e34
Update phpunit/html/wp-html-walker-test.php
adamziel 1021d9b
Update phpunit/html/wp-html-walker-test.php
adamziel e0910be
Explain the test_set_attribute_takes_priority_over_add_class test
adamziel 273c502
Merge branch 'add/html-tokenizer-2' of github.com:WordPress/gutenberg…
adamziel 19a8546
Rename test_works_with_single_quote_marks to test_parses_html_attribu…
adamziel c96f08b
Update the test names to suggest the failure mode
adamziel 218afe4
Update phpunit/html/wp-html-walker-test.php
adamziel 81d43e2
Add covers annotations
adamziel 53143f8
Rename WP_HTML_Walker to WP_HTML_Tag_Processor
adamziel bddc2db
Remove the is_closed method, as the idea of a closed walker does not …
adamziel f299bf0
Update lib/experimental/html/class-wp-html-text-replacement.php
adamziel 13c3270
Update lib/experimental/html/class-wp-html-attribute-token.php
adamziel 5c2d080
Rename $w to $p
adamziel 509684c
Merge branch 'add/html-tokenizer-2' of github.com:WordPress/gutenberg…
adamziel d9d3bab
Rename phpdoc references to $w to $p
adamziel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
<?php | ||
/** | ||
* HTML Tag Processor: Attribute token structure class. | ||
* | ||
* @package WordPress | ||
* @subpackage HTML | ||
* @since 6.1.0 | ||
*/ | ||
|
||
/** | ||
* Data structure for the attribute token that allows to drastically improve performance. | ||
* | ||
* This class is for internal usage of the WP_HTML_Tag_Processor class. | ||
* | ||
* @access private | ||
* @since 6.1.0 | ||
adamziel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* | ||
* @see WP_HTML_Tag_Processor | ||
*/ | ||
class WP_HTML_Attribute_Token { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please lock down this code so it is not accessible from plugins or other parts of WordPress. It should only be used for the cases described in this PR. |
||
/** | ||
* Attribute name. | ||
* | ||
* @since 6.1.0 | ||
* @var string | ||
*/ | ||
public $name; | ||
|
||
/** | ||
* Attribute value. | ||
* | ||
* @since 6.1.0 | ||
* @var int | ||
*/ | ||
public $value_starts_at; | ||
|
||
/** | ||
* How many bytes the value occupies in the input HTML. | ||
* | ||
* @since 6.1.0 | ||
* @var int | ||
*/ | ||
public $value_length; | ||
|
||
/** | ||
* The string offset where the attribute name starts. | ||
* | ||
* @since 6.1.0 | ||
* @var int | ||
*/ | ||
public $start; | ||
|
||
/** | ||
* The string offset after the attribute value or its name. | ||
* | ||
* @since 6.1.0 | ||
* @var int | ||
*/ | ||
public $end; | ||
|
||
/** | ||
* Whether the attribute is a boolean attribute with value `true`. | ||
* | ||
* @since 6.1.0 | ||
* @var bool | ||
*/ | ||
public $is_true; | ||
|
||
/** | ||
* Constructor. | ||
* | ||
* @since 6.1.0 | ||
* | ||
* @param string $name Attribute name. | ||
* @param int $value_start Attribute value. | ||
* @param int $value_length Number of bytes attribute value spans. | ||
* @param int $start The string offset where the attribute name starts. | ||
* @param int $end The string offset after the attribute value or its name. | ||
* @param bool $is_true Whether the attribute is a boolean attribute with true value. | ||
*/ | ||
public function __construct( $name, $value_start, $value_length, $start, $end, $is_true ) { | ||
$this->name = $name; | ||
$this->value_starts_at = $value_start; | ||
$this->value_length = $value_length; | ||
$this->start = $start; | ||
$this->end = $end; | ||
$this->is_true = $is_true; | ||
} | ||
} |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamziel and @dmsnell, is the note about the performance implications valid? Should we also include the same note for
WP_Class_Name_Update
andWP_Text_Replacement
? I know you discussed that extensively, but I'm not sure which part made the biggest difference.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is valid, I think in @dmsnell's testing it reduced memory usage by like 90% compared to
array()
which sounds almost unbelievable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we can update these comments across the board.
array()
was surprisingly terribly inefficient with memory use.