Skip to content

Commit 9f5a895

Browse files
committed
Squash merge #7649
Squashed commit of the following: commit 1cde425 Merge: 05ca2a4 8ad5281 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 20:40:05 2024 +0100 Merge branch 'trunk' into html-api/fix-63390-seek commit 05ca2a4 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 20:29:36 2024 +0100 Remove context-node cases from open elements The context node is not pushed to the stack of open elements, and therefore does not need special handling. commit 75ab9c2 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 20:27:57 2024 +0100 Break at root-node when popping elements on seek The context node is not pushed to the stack of open elements, this code was either doing nothing or incorrect. commit 90eb6e2 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 13:53:15 2024 +0100 Use a temporary bookmark to avoid modifying tag processor internal state commit b95e402 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 13:45:13 2024 +0100 Restore private stack properties to private commit d5a7d5c Author: Jon Surrell <[email protected]> Date: Wed Nov 6 13:40:18 2024 +0100 Rework with tests for fragment and full processors Use a dataprovider to get a factory function for processors commit f7af6e3 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 13:19:58 2024 +0100 Add test seeking from SVG namespace commit 388bf19 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 13:05:34 2024 +0100 Add and improve HTML processor bookmark tests commit d5bf14c Author: Jon Surrell <[email protected]> Date: Wed Nov 6 12:17:47 2024 +0100 Add and improve comments commit d181938 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 12:13:48 2024 +0100 Fix fragment state reset for different context nodes commit 9a0c017 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 12:05:43 2024 +0100 Set parsing namespace correctly when seeking commit 660dc85 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 11:49:21 2024 +0100 Remove condition that may not be necessary This seems related to virutal tokens and it likely covered by the virtual token condition commit 9af204c Author: Jon Surrell <[email protected]> Date: Wed Nov 6 11:47:56 2024 +0100 Assert setting bookmark two This is consisten with the assertion on bookmark one commit 851df38 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 11:47:17 2024 +0100 Fix fragment parser bug that should reset breadcrumbs before seeking commit 6106a56 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 11:46:49 2024 +0100 Use do-while loop to iterate after first check commit 744cf01 Author: Jon Surrell <[email protected]> Date: Wed Nov 6 11:42:42 2024 +0100 Add commentary around state resetting in seek commit cb63bbc Author: Jon Surrell <[email protected]> Date: Tue Nov 5 19:51:29 2024 +0100 Lint: remove empty line commit 737bf92 Author: Jon Surrell <[email protected]> Date: Tue Nov 5 19:49:01 2024 +0100 Restore original bookmark matching commit 89aa01b Author: Jon Surrell <[email protected]> Date: Tue Nov 5 19:47:16 2024 +0100 Fix the issue commit dc3f1e6 Author: Jon Surrell <[email protected]> Date: Tue Nov 5 19:06:23 2024 +0100 try fixes commit 541b4f6 Author: Jon Surrell <[email protected]> Date: Fri Oct 25 23:19:19 2024 +0200 wip commit 5d75bb7 Author: Jon Surrell <[email protected]> Date: Fri Oct 25 23:15:51 2024 +0200 Work on fix commit f9777aa Author: Jon Surrell <[email protected]> Date: Fri Oct 25 22:51:44 2024 +0200 Add failing test case
1 parent 27a9781 commit 9f5a895

File tree

4 files changed

+201
-35
lines changed

4 files changed

+201
-35
lines changed

src/wp-includes/html-api/class-wp-html-open-elements.php

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -520,11 +520,6 @@ public function pop(): bool {
520520
return false;
521521
}
522522

523-
if ( 'context-node' === $item->bookmark_name ) {
524-
$this->stack[] = $item;
525-
return false;
526-
}
527-
528523
$this->after_element_pop( $item );
529524
return true;
530525
}
@@ -585,10 +580,6 @@ public function push( WP_HTML_Token $stack_item ): void {
585580
* @return bool Whether the node was found and removed from the stack of open elements.
586581
*/
587582
public function remove_node( WP_HTML_Token $token ): bool {
588-
if ( 'context-node' === $token->bookmark_name ) {
589-
return false;
590-
}
591-
592583
foreach ( $this->walk_up() as $position_from_end => $item ) {
593584
if ( $token->bookmark_name !== $item->bookmark_name ) {
594585
continue;

src/wp-includes/html-api/class-wp-html-processor.php

Lines changed: 57 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5409,51 +5409,83 @@ public function seek( $bookmark_name ): bool {
54095409
*/
54105410
if ( 'backward' === $direction ) {
54115411
/*
5412-
* Instead of clearing the parser state and starting fresh, calling the stack methods
5413-
* maintains the proper flags in the parser.
5412+
* When moving backward, stateful stacks should be cleared.
54145413
*/
54155414
foreach ( $this->state->stack_of_open_elements->walk_up() as $item ) {
5416-
if ( 'context-node' === $item->bookmark_name ) {
5415+
/*
5416+
* Fragment parsers always start with an HTML root node at the top of the stack.
5417+
* Do not remove it.
5418+
*/
5419+
if ( 'root-node' === $item->bookmark_name ) {
54175420
break;
54185421
}
54195422

54205423
$this->state->stack_of_open_elements->remove_node( $item );
54215424
}
54225425

54235426
foreach ( $this->state->active_formatting_elements->walk_up() as $item ) {
5424-
if ( 'context-node' === $item->bookmark_name ) {
5425-
break;
5426-
}
5427-
54285427
$this->state->active_formatting_elements->remove_node( $item );
54295428
}
54305429

5431-
parent::seek( 'context-node' );
5432-
$this->state->insertion_mode = WP_HTML_Processor_State::INSERTION_MODE_IN_BODY;
5433-
$this->state->frameset_ok = true;
5434-
$this->element_queue = array();
5435-
$this->current_element = null;
5430+
/*
5431+
* **After** clearing stacks, more processor state can be reset.
5432+
* This must be done after clearing the stack because those stacks generate events that
5433+
* would appear on a subsequent call to `next_token()`.
5434+
*/
5435+
$this->state->frameset_ok = true;
5436+
$this->state->stack_of_template_insertion_modes = array();
5437+
$this->state->head_element = null;
5438+
$this->state->form_element = null;
5439+
$this->state->current_token = null;
5440+
$this->current_element = null;
5441+
$this->element_queue = array();
5442+
5443+
/*
5444+
* The absence of a context node indicates a full parse.
5445+
* The presence of a context node indicates a fragment parser.
5446+
*/
5447+
if ( null === $this->context_node ) {
5448+
$this->change_parsing_namespace( 'html' );
5449+
$this->state->insertion_mode = WP_HTML_Processor_State::INSERTION_MODE_INITIAL;
5450+
$this->breadcrumbs = array();
54365451

5437-
if ( isset( $this->context_node ) ) {
5438-
$this->breadcrumbs = array_slice( $this->breadcrumbs, 0, 2 );
5452+
$this->bookmarks['initial'] = new WP_HTML_Span( 0, 0 );
5453+
parent::seek( 'initial' );
5454+
unset( $this->bookmarks['initial'] );
54395455
} else {
5440-
$this->breadcrumbs = array();
5441-
}
5442-
}
5456+
$this->change_parsing_namespace(
5457+
$this->context_node->integration_node_type
5458+
? 'html'
5459+
: $this->context_node->namespace
5460+
);
5461+
5462+
if ( 'TEMPLATE' === $this->context_node->node_name ) {
5463+
$this->state->stack_of_template_insertion_modes[] = WP_HTML_Processor_State::INSERTION_MODE_IN_TEMPLATE;
5464+
}
54435465

5444-
// When moving forwards, reparse the document until reaching the same location as the original bookmark.
5445-
if ( $bookmark_starts_at === $this->bookmarks[ $this->state->current_token->bookmark_name ]->start ) {
5446-
return true;
5466+
$this->reset_insertion_mode_appropriately();
5467+
$this->breadcrumbs = array_slice( $this->breadcrumbs, 0, 2 );
5468+
parent::seek( $this->context_node->bookmark_name );
5469+
}
54475470
}
54485471

5449-
while ( $this->next_token() ) {
5472+
/*
5473+
* Here, the processor moves forward through the document until it matches the bookmark.
5474+
* do-while is used here because the processor is expected to already be stopped on
5475+
* a token than may match the bookmarked location.
5476+
*/
5477+
do {
5478+
/*
5479+
* The processor will stop on virtual tokens, but bookmarks may not be set on them.
5480+
* They should not be matched when seeking a bookmark, skip them.
5481+
*/
5482+
if ( $this->is_virtual() ) {
5483+
continue;
5484+
}
54505485
if ( $bookmark_starts_at === $this->bookmarks[ $this->state->current_token->bookmark_name ]->start ) {
5451-
while ( isset( $this->current_element ) && WP_HTML_Stack_Event::POP === $this->current_element->operation ) {
5452-
$this->current_element = array_shift( $this->element_queue );
5453-
}
54545486
return true;
54555487
}
5456-
}
5488+
} while ( $this->next_token() );
54575489

54585490
return false;
54595491
}
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
<?php
2+
/**
3+
* Unit tests covering WP_HTML_Processor bookmark functionality.
4+
*
5+
* @package WordPress
6+
* @subpackage HTML-API
7+
*/
8+
9+
/**
10+
* @group html-api
11+
*
12+
* @coversDefaultClass WP_HTML_Processor
13+
*/
14+
class Tests_HtmlApi_WpHtmlProcessor_Bookmark extends WP_UnitTestCase {
15+
/**
16+
* @dataProvider data_processor_constructors
17+
*
18+
* @ticket 62290
19+
*/
20+
public function test_processor_seek_same_location( callable $factory ) {
21+
$processor = $factory( '<div><span>' );
22+
$this->assertTrue( $processor->next_tag( 'DIV' ) );
23+
$this->assertTrue( $processor->set_bookmark( 'mark' ), 'Failed to set bookmark.' );
24+
$this->assertTrue( $processor->has_bookmark( 'mark' ), 'Failed has_bookmark check.' );
25+
26+
// Confirm the bookmark works and processing continues normally.
27+
$this->assertTrue( $processor->seek( 'mark' ), 'Failed to seek to bookmark.' );
28+
$this->assertSame( 'DIV', $processor->get_tag() );
29+
$this->assertSame( array( 'HTML', 'BODY', 'DIV' ), $processor->get_breadcrumbs() );
30+
$this->assertTrue( $processor->next_tag() );
31+
$this->assertSame( 'SPAN', $processor->get_tag() );
32+
$this->assertSame( array( 'HTML', 'BODY', 'DIV', 'SPAN' ), $processor->get_breadcrumbs() );
33+
}
34+
35+
/**
36+
* @dataProvider data_processor_constructors
37+
*
38+
* @ticket 62290
39+
*/
40+
public function test_processor_seek_backward( callable $factory ) {
41+
$processor = $factory( '<div><span>' );
42+
$this->assertTrue( $processor->next_tag( 'DIV' ) );
43+
$this->assertTrue( $processor->set_bookmark( 'mark' ), 'Failed to set bookmark.' );
44+
$this->assertTrue( $processor->has_bookmark( 'mark' ), 'Failed has_bookmark check.' );
45+
46+
// Move past the bookmark so it must scan backwards.
47+
$this->assertTrue( $processor->next_tag( 'SPAN' ) );
48+
49+
// Confirm the bookmark works.
50+
$this->assertTrue( $processor->seek( 'mark' ), 'Failed to seek to bookmark.' );
51+
$this->assertSame( 'DIV', $processor->get_tag() );
52+
}
53+
54+
/**
55+
* @dataProvider data_processor_constructors
56+
*
57+
* @ticket 62290
58+
*/
59+
public function test_processor_seek_forward( callable $factory ) {
60+
$processor = $factory( '<div one></div><span two></span><a three>' );
61+
$this->assertTrue( $processor->next_tag( 'DIV' ) );
62+
$this->assertTrue( $processor->set_bookmark( 'one' ), 'Failed to set bookmark "one".' );
63+
$this->assertTrue( $processor->has_bookmark( 'one' ), 'Failed "one" has_bookmark check.' );
64+
65+
// Move past the bookmark so it must scan backwards.
66+
$this->assertTrue( $processor->next_tag( 'SPAN' ) );
67+
$this->assertTrue( $processor->get_attribute( 'two' ) );
68+
$this->assertTrue( $processor->set_bookmark( 'two' ), 'Failed to set bookmark "two".' );
69+
$this->assertTrue( $processor->has_bookmark( 'two' ), 'Failed "two" has_bookmark check.' );
70+
71+
// Seek back.
72+
$this->assertTrue( $processor->seek( 'one' ), 'Failed to seek to bookmark "one".' );
73+
$this->assertSame( 'DIV', $processor->get_tag() );
74+
75+
// Seek forward and continue processing.
76+
$this->assertTrue( $processor->seek( 'two' ), 'Failed to seek to bookmark "two".' );
77+
$this->assertSame( 'SPAN', $processor->get_tag() );
78+
$this->assertTrue( $processor->get_attribute( 'two' ) );
79+
80+
$this->assertTrue( $processor->next_tag() );
81+
$this->assertSame( 'A', $processor->get_tag() );
82+
$this->assertTrue( $processor->get_attribute( 'three' ) );
83+
}
84+
85+
/**
86+
* Ensure the parsing namespace is handled when seeking from foreign content.
87+
*
88+
* @dataProvider data_processor_constructors
89+
*
90+
* @ticket 62290
91+
*/
92+
public function test_seek_back_from_foreign_content( callable $factory ) {
93+
$processor = $factory( '<custom-element /><svg><rect />' );
94+
$this->assertTrue( $processor->next_tag( 'CUSTOM-ELEMENT' ) );
95+
$this->assertTrue( $processor->set_bookmark( 'mark' ), 'Failed to set bookmark "mark".' );
96+
$this->assertTrue( $processor->has_bookmark( 'mark' ), 'Failed "mark" has_bookmark check.' );
97+
98+
/*
99+
* <custom-element /> has self-closing flag, but HTML elements (that are not void elements) cannot self-close,
100+
* they must be closed by some means, usually a closing tag.
101+
*
102+
* If the div were interpreted as foreign content, it would self-close.
103+
*/
104+
$this->assertTrue( $processor->has_self_closing_flag() );
105+
$this->assertTrue( $processor->expects_closer(), 'Incorrectly interpreted HTML custom-element with self-closing flag as self-closing element.' );
106+
107+
// Proceed into foreign content.
108+
$this->assertTrue( $processor->next_tag( 'RECT' ) );
109+
$this->assertSame( 'svg', $processor->get_namespace() );
110+
$this->assertTrue( $processor->has_self_closing_flag() );
111+
$this->assertFalse( $processor->expects_closer() );
112+
$this->assertSame( array( 'HTML', 'BODY', 'CUSTOM-ELEMENT', 'SVG', 'RECT' ), $processor->get_breadcrumbs() );
113+
114+
// Seek back.
115+
$this->assertTrue( $processor->seek( 'mark' ), 'Failed to seek to bookmark "mark".' );
116+
$this->assertSame( 'CUSTOM-ELEMENT', $processor->get_tag() );
117+
// If the parsing namespace were not correct here (html),
118+
// then the self-closing flag would be misinterpreted.
119+
$this->assertTrue( $processor->has_self_closing_flag() );
120+
$this->assertTrue( $processor->expects_closer(), 'Incorrectly interpreted HTML custom-element with self-closing flag as self-closing element.' );
121+
122+
// Proceed into foreign content again.
123+
$this->assertTrue( $processor->next_tag( 'RECT' ) );
124+
$this->assertSame( 'svg', $processor->get_namespace() );
125+
$this->assertTrue( $processor->has_self_closing_flag() );
126+
$this->assertFalse( $processor->expects_closer() );
127+
128+
// The RECT should still descend from the CUSTOM-ELEMENT despite its self-closing flag.
129+
$this->assertSame( array( 'HTML', 'BODY', 'CUSTOM-ELEMENT', 'SVG', 'RECT' ), $processor->get_breadcrumbs() );
130+
}
131+
132+
/**
133+
* Data provider.
134+
*
135+
* @return array
136+
*/
137+
public static function data_processor_constructors(): array {
138+
return array(
139+
'Full parser' => array( array( WP_HTML_Processor::class, 'create_full_parser' ) ),
140+
'Fragment parser' => array( array( WP_HTML_Processor::class, 'create_fragment' ) ),
141+
);
142+
}
143+
}

tests/phpunit/tests/html-api/wpHtmlProcessor.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public function test_clear_to_navigate_after_seeking() {
133133

134134
// Create a bookmark inside of that stack.
135135
if ( null !== $processor->get_attribute( 'two' ) ) {
136-
$processor->set_bookmark( 'two' );
136+
$this->assertTrue( $processor->set_bookmark( 'two' ) );
137137
break;
138138
}
139139
}

0 commit comments

Comments
 (0)