HTML API: Implement adoption agency algorithm and active format reconstruction#81
HTML API: Implement adoption agency algorithm and active format reconstruction#81sirreal wants to merge 5 commits into
Conversation
Introduce the supporting operations that the adoption agency algorithm and active formatting element reconstruction require on the two parser stacks. On the stack of open elements: - Extract the "in scope" element list into a shared class constant. - Add `has_node_in_scope()`, which reports whether a specific node (rather than any element of a given tag name) is in scope. The adoption agency algorithm must test a specific formatting element, regardless of other open elements sharing its tag name. On the list of active formatting elements, add position-indexed operations (`position_of()`, `remove_at()`, `insert_at()`, `replace_node()`) so entries can be cloned and replaced in place as the algorithms direct. These additions are unused until the algorithms are implemented and do not change parsing behavior.
Implement the adoption agency algorithm and active formatting element reconstruction so the HTML Processor handles misnested formatting elements instead of bailing. Previously the processor stopped whenever a document required reconstructing implicitly-closed formatting elements (e.g. `<p><b>1<p>2`) or running the adoption agency algorithm (e.g. `<b>1<p>2</b>3`). Both are now supported: - `reconstruct_active_formatting_elements()` reopens the run of unclosed formatting elements at the end of the list, per the specification's rewind/advance/create steps. - `run_adoption_agency_algorithm()` implements the full algorithm, including the furthest-block case and the "any other end tag" fallback. - The "Noah's Ark clause" limits the list of active formatting elements to three equivalent entries (same tag name, namespace, and attributes). Because the processor visits a document in a single pass, it cannot relocate nodes it has already reported. The parser's state (the stack of open elements and the list of active formatting elements) is maintained exactly as the specification requires, so every token visited after these algorithms run is reported with the ancestor chain a browser would produce. Nodes which were already visited when a misnesting is discovered remain where they were found. Formatting elements reopened by the parser are reported as "virtual" nodes. Reading an attribute, class, or qualified name of such a node reports the value from the tag which opened the original element; these nodes cannot be modified. Supporting this required hardening stack-event provenance so a single source tag never produces two visitor events: pushes are matched to the current token by identity, and each tag closer is matched to at most one popped node. The html5lib test cases whose constructed trees differ only because the adoption agency algorithm re-parents already-visited nodes are skip-listed with a shared reason; each was verified to match browser behavior for parser state and normalization. The absorbed `wpHtmlSupportRequiredActiveFormatReconstruction` test and the previous bail-asserting cases are replaced with tests of the new behavior.
A FORM end tag encountered while other elements remain open no longer stops the parser. The form element is removed from the stack of open elements using the same reconciliation the adoption agency algorithm uses, so any elements that remain open after it are reported with correct breadcrumbs. The scope check now tests the specific form element pointer rather than any FORM element in scope, matching the specification. One html5lib case (`<form><div></form><div>`) exercises a shape a single-pass token stream cannot represent: browsers keep the closed FORM as a DOM ancestor of its still-open descendants. This parser reports following content outside the closed FORM, mirroring the stack of open elements; the case is skip-listed with that reason.
|
Agent's report @ 764adbd:
|
|
Found one API behavior issue:
Example: $p = WP_HTML_Processor::create_fragment( '<p><b class=bold>inside</p>outside' );
while ( $p->next_tag( array( 'tag_name' => 'B', 'class_name' => 'bold', 'tag_closers' => 'visit' ) ) ) {
var_dump( $p->is_tag_closer(), $p->get_attribute( 'class' ) );
}This matches the virtual |
sirreal
left a comment
There was a problem hiding this comment.
Review
Cross-checked run_adoption_agency_algorithm() and reconstruct_active_formatting_elements() step-by-step against the WHATWG spec text, and verified locally: --group html-api passes (1,506 tests), the html5lib suite gains 35 newly-passing cases vs the base commit (1,123 vs 1,088, runtime bails 340 vs 420), and PHPCS is clean on all changed files. The bookmark-position adjustments in the adoption agency inner loop, the working-stack index bookkeeping, and the reconcile-then-reopen event ordering all check out against the spec, and the pinned event-stream test (test_adoption_agency_event_stream_remains_properly_nested) is exactly the right regression guard for the reconciliation design. Rewriting stack-event provenance to token identity is strictly better than the old tag-name heuristic, and the added 'html' === $current_node->namespace check on the adoption-agency shortcut fixes a latent spec mismatch in the old code.
Findings
1. Virtual tag closers report their opener's attributes; real closers report none.
A real </b> returns null from get_attribute() (end tags have no attributes), but the virtual closer of a cloned or reconstructed formatting element returns the source opener's attributes, because the pop event carries the same token whose bookmark spans the opening tag:
$p = WP_HTML_Processor::create_fragment( '<p><b class="bold">inside</p>outside' );
while ( $p->next_token() ) {
if ( 'B' === $p->get_token_name() && $p->is_tag_closer() ) {
var_dump( $p->get_attribute( 'class' ) ); // string(4) "bold" — a real </b> gives NULL
}
}A consumer matching on attributes while walking tokens will now match virtual close events but never real ones. Suggest having get_virtual_node_attribute_reader() return null when the current element is a WP_HTML_Stack_Event::POP so closers behave uniformly.
2. Missing @since changelog notes. get_attribute(), get_attribute_names_with_prefix(), and class_list() all gained the same virtual-node behavior as has_class(), but only has_class() documents it with @since 7.1.0 Reports class names for reconstructed formatting elements…. Add matching notes to the other three.
3. The FORM divergence narrows a documented guarantee but isn't in the class-level docs. The run_adoption_agency_algorithm() docblock promises that "every token visited after this algorithm runs is reported with the ancestor chain a browser would report for it" — true for adoption, but the FORM change introduces the one case where following tokens get non-browser breadcrumbs (a browser keeps the closed FORM as ancestor of subsequent content). Today that's only documented in the html5lib skip constant (SKIP_HTML_PARSER_CANNOT_HOLD_FORM_OPEN), where get_breadcrumbs() consumers won't find it. Worth a sentence in the WP_HTML_Processor class docblock alongside the adoption/fostering discussion.
4. Noah's Ark signature reader skips change_parsing_namespace(). get_virtual_node_attribute_reader() sets the parsing namespace on its reader; get_attribute_comparison_signature() doesn't. Harmless today since the list of active formatting elements only ever holds HTML formatting elements, but either add the one-liner or a comment saying why it's unnecessary, so the asymmetry doesn't read as an oversight.
5. @ticket 58517 on the new tests. That ticket closed as fixed in 6.4; this work will presumably land under #65383 (the 7.1 HTML API tracking ticket). Consider annotating the new tests with the ticket the change actually lands under.
None of these block the approach — #1 is the only behavioral one and it's a one-line guard.
Implements two of the HTML Processor's longest-standing unsupported features: reconstruction of active formatting elements and the adoption agency algorithm. The processor no longer bails on misnested or implicitly-closed formatting elements, e.g.
<b>bold <i>both</b> italic</i>or<p><em>One<p><em>Two.What's here
Three commits, intended to be reviewed in order:
Stack helpers — positional primitives on
WP_HTML_Active_Formatting_Elements(position_of,remove_at,insert_at,replace_node) which the adoption agency algorithm requires, andWP_HTML_Open_Elements::has_node_in_scope()for the spec steps which ask whether a specific node is in scope rather than any element with a given tag name. The "element in scope" termination list moves to a shared class constant.Adoption agency + active format reconstruction — removes the
bail()calls inreconstruct_active_formatting_elements()andrun_adoption_agency_algorithm()and implements both algorithms fully, including the "Noah's Ark clause" (push_onto_active_formatting_elements()) with real attribute comparison. Real/virtual stack-event provenance is rewritten to use token identity instead of tag-name matching, which the adoption agency algorithm demands (a single tag closer may pop several same-named elements; only one of them is the tag found in the input).FORM end tags —
</form>no longer bails when other elements remain open inside the form.Design: single-pass constraints
A streaming parser visits each node once and cannot move nodes it has already reported. This drives every deviation from browser behavior, and each one is documented at its site:
reconcile_stack_of_open_elements()), so the token stream never misnests.get_attribute(),get_attribute_names_with_prefix(),get_qualified_attribute_name(),has_class(), andclass_list()read the attributes of the tag which created the original element (via a lazily-created, cached reader over the source span). Modification (set_attribute(),remove_attribute(), etc.) remains refused, since there is no tag in the input HTML to modify.Testing
Tests_HtmlApi_WpHtmlProcessorActiveFormattingElementscovers: reconstruction across paragraphs and on text nodes, attribute reads on reconstructed elements (including write refusal), the Noah's Ark clause with and without attribute comparison, adoption with and without a furthest block, deep misnesting, the "any other end tag" fallback, A-implicitly-closes-A, a pinned event stream for an adoption case, andseek()stability across an adoption boundary.SKIP_HTML_PARSER_REPARENTS_VISITED_NODES,SKIP_HTML_PARSER_CANNOT_HOLD_FORM_OPEN).phpunit --group html-api: 1,506 tests pass. PHPCS clean on all changed files.Trac ticket: https://core.trac.wordpress.org/ticket/65383
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Fable 5
Used for: Implementation.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.