-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Block Bindings: Allow more generic setting of block attributes #9469
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
Changes from 1 commit
7f1f6a0
054756d
b328dee
21e80e2
1c70bc3
8f76759
b7b7ca5
bb7c906
103a5c4
3f2e32b
85b6354
b67890e
6bc4fd5
d943cd8
527c5d8
86e836d
dad7380
88af5ff
a56f978
7c3fc45
66fef38
24a0b0d
2e7df73
e2f0a38
b045050
e739c6c
6a4a3a3
4dbb0e5
a18e435
3410f40
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -462,50 +462,47 @@ private function replace_html( string $block_content, string $attribute_name, $s | |
| } | ||
|
|
||
| private static function get_block_bindings_processor( string $block_content ) { | ||
| static $internal_processor_class = null; | ||
| if ( null === $internal_processor_class ) { | ||
| $internal_processor_class = new class('', WP_HTML_Processor::CONSTRUCTOR_UNLOCK_CODE) extends WP_HTML_Processor { | ||
| /** | ||
| * Replace the rich text content between a tag opener and matching closer. | ||
| * | ||
| * When stopped on a tag opener, replace the content enclosed by it and its | ||
| * matching closer with the provided rich text. | ||
| * | ||
| * @param string $rich_text The rich text to replace the original content with. | ||
| * @return bool True on success. | ||
| */ | ||
| public function replace_rich_text( $rich_text ) { | ||
| if ( $this->is_tag_closer() ) { | ||
| return false; | ||
| } | ||
| $internal_processor_class = new class('', WP_HTML_Processor::CONSTRUCTOR_UNLOCK_CODE) extends WP_HTML_Processor { | ||
| /** | ||
| * Replace the rich text content between a tag opener and matching closer. | ||
| * | ||
| * When stopped on a tag opener, replace the content enclosed by it and its | ||
| * matching closer with the provided rich text. | ||
| * | ||
| * @param string $rich_text The rich text to replace the original content with. | ||
| * @return bool True on success. | ||
| */ | ||
| public function replace_rich_text( $rich_text ) { | ||
| if ( $this->is_tag_closer() ) { | ||
| return false; | ||
| } | ||
|
|
||
| $depth = $this->get_current_depth(); | ||
| $depth = $this->get_current_depth(); | ||
|
|
||
| $this->set_bookmark( '_wp_block_bindings_tag_opener' ); | ||
| // The bookmark names are prefixed with `_` so the key below has an extra `_`. | ||
| $tag_opener = $this->bookmarks['__wp_block_bindings_tag_opener']; | ||
| $start = $tag_opener->start + $tag_opener->length; | ||
| $this->release_bookmark( '_wp_block_bindings_tag_opener' ); | ||
| $this->set_bookmark( '_wp_block_bindings_tag_opener' ); | ||
| // The bookmark names are prefixed with `_` so the key below has an extra `_`. | ||
| $tag_opener = $this->bookmarks['__wp_block_bindings_tag_opener']; | ||
| $start = $tag_opener->start + $tag_opener->length; | ||
| $this->release_bookmark( '_wp_block_bindings_tag_opener' ); | ||
|
|
||
| // Find matching tag closer. | ||
| while ( $this->next_token() && $this->get_current_depth() >= $depth ) { | ||
| } | ||
| // Find matching tag closer. | ||
| while ( $this->next_token() && $this->get_current_depth() >= $depth ) { | ||
| } | ||
|
Member
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. reasonable follow-up idea: verify that the processor completed and didn’t pause at an incomplete token or reach unsupported markup.
Contributor
Author
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.
Member
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. looks good; I left a comment on the new PR. |
||
|
|
||
| $this->set_bookmark( '_wp_block_bindings_tag_closer' ); | ||
| $tag_closer = $this->bookmarks['__wp_block_bindings_tag_closer']; | ||
| $end = $tag_closer->start; | ||
| $this->release_bookmark( '_wp_block_bindings_tag_closer' ); | ||
| $this->set_bookmark( '_wp_block_bindings_tag_closer' ); | ||
| $tag_closer = $this->bookmarks['__wp_block_bindings_tag_closer']; | ||
| $end = $tag_closer->start; | ||
| $this->release_bookmark( '_wp_block_bindings_tag_closer' ); | ||
|
|
||
| $this->lexical_updates[] = new WP_HTML_Text_Replacement( | ||
| $start, | ||
| $end - $start, | ||
| $rich_text | ||
| ); | ||
| $this->lexical_updates[] = new WP_HTML_Text_Replacement( | ||
| $start, | ||
| $end - $start, | ||
| $rich_text | ||
| ); | ||
|
Member
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. applying lexical updates that span token boundaries is not something we have particularly tested well. it’s probably working well enough and hopefully the tests here will catch them, but this is sort of uncharted territory within the HTML Processor. we should make sure that we don’t seek anywhere here to verify that we don’t cross indices. this single issue is one of the main reasons I proposed the single linear pass of the serialization builder, because it structurally prevents the ability of jumping around or referring to indices after changing positions. I’m not particularly confident I would know how this interacts with virtual tokens, for example, whose boundaries are coincident with bookmarks (vs. realized tokens which all have non-overlapping spans) |
||
|
|
||
| return true; | ||
| } | ||
| }; | ||
| } | ||
| return true; | ||
| } | ||
| }; | ||
|
|
||
| return $internal_processor_class::create_fragment( $block_content ); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
this is certainly welcome to do, to release the bookmarks, but performance-wise it’s not setting a better example than leaving them dangling.
given that this function contains all of the scanning, it’s even fine to trap the bookmark bounds and re-use one, such as
set_bookmark( 'here' ). I’ve considered exposing something like->current_token()but haven’t yet.part of this is the fact that we expect isolation of bookmarks here and we will dismiss the processor when we’re done.