Ignore stray closing P tags when tracking the open element stack

[westonruter]

Summary

Fixes #2650

wpautop() wraps two [caption] shortcodes that share a paragraph in a single P, and do_shortcode() then expands each into a FIGURE:

<p><figure class="wp-caption">…</figure><br />
<figure class="wp-caption">…</figure></p>

OD_HTML_Tag_Processor already handles the first half correctly: FIGURE is in self::P_CLOSING_TAGS, so the opening P is implicitly closed. It got the second half wrong. The now-stray </p> fell through to the generic closer branch and popped unconditionally, taking the enclosing DIV off the stack.

Per the HTML spec, an end tag for P with no P element in scope closes nothing — a parse error is raised and an empty P element is implied at that position instead. Confirmed against WP_HTML_Processor on the same fragment:

spec:  DIV > P, DIV > FIGURE, DIV > FIGURE, DIV > P (implied), BODY > FOOTER
before: DIV > P, DIV > FIGURE, DIV > FIGURE,                   FOOTER  ← DIV consumed

Why this matters beyond the log noise

The stack stayed one level short for the remainder of the document, so every XPath computed after the stray closer was wrong. On the homepage of my test site, 8 of the 12 data-od-xpath values lost the MAIN level entirely and picked up the wrong ARTICLE index:

before: …/*[1][self::DIV]/*[2][self::ARTICLE]/*[2][self::DIV]/*[1][self::FIGURE]/…
after:  …/*[1][self::DIV]/*[1][self::MAIN]/*[9][self::ARTICLE]/*[2][self::DIV]/*[1][self::FIGURE]/…

XPaths are the keys used to match URL Metrics back to elements, so no element after the stray closer could be matched and the optimizations depending on those matches silently did not apply.

Relevant technical choices

Advancing the sibling index is not cosmetic. The browser's DOM does contain the implied empty P, so it occupies a sibling slot. Without the bump, a following sibling computes *[3] where the DOM has it at position 4, and the mismatch just moves rather than disappears. The new snapshot case pins this: the hero image after the stray closer resolves to *[6][self::IMG], matching the XPath a browser reports, so the stored LCP element matches and fetchpriority="high" is applied. Before this change it computed *[5], the lookup missed, and no attribute was added.

The guard is deliberately narrow. It fires only when there is no open P anywhere on the stack — a genuinely stray closer. Shapes such as <p><em></p>, where a closer should pop more than one element, keep their existing behavior; that broader divergence between our rudimentary stack tracking and the spec is #2622, and #1546 remains the wholesale alternative.

Out of scope. This fixes the input that was emitting notices, not the fact that notices raised inside the output buffer callback are swallowed. On PHP 8.5 those surface as:

Deprecated: ob_end_flush(): Producing output from user output handler {closure:od_buffer_output():56} is deprecated
Deprecated: ob_end_flush(): Producing output from user output handler {closure:Perflab_Server_Timing::start_output_buffer():277} is deprecated

because PHP 8.5 deprecates and discards output produced from within a user output handler — which is why the underlying notice was invisible in the browser and only reached debug.log. Adopting core's template enhancement output buffer, which installs its own error handler, addresses that: #2224 / #2516 for Optimization Detective, #2225 for Server-Timing.

Testing

• New stray-closing-p case in data_provider_sample_documents, asserting FOOTER lands at *[6].
• New stray-closing-p snapshot test case exercising the URL Metric match end to end.
• All 13 existing snapshots are unchanged — no actual.html was regenerated for any of them.
• optimization-detective: 358 tests, 12,323 assertions. image-prioritizer: 91 tests. embed-optimizer: 66 tests.
• phpcs and phpstan clean.
• On the test site the homepage goes from 4 tag stack warnings (plus the two deprecations) to zero, and all 12 image XPaths regain the MAIN level.

Use of AI Tools

Claude Opus 5, via Claude Code, did the root-cause analysis, wrote the patch, and wrote the tests. I reviewed the diagnosis and the change, and take responsibility for it. The analysis was cross-checked against WP_HTML_Processor as the spec-compliant reference rather than taken on assertion.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: b1ink0 <b1ink0@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.50%. Comparing base (cedbbac) to head (1c33f32).
⚠️ Report is 19 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...mization-detective/class-od-html-tag-processor.php	50.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk    #2651      +/-   ##
==========================================
+ Coverage   70.35%   70.50%   +0.15%     
==========================================
  Files          91       91              
  Lines        7867     7870       +3     
==========================================
+ Hits         5535     5549      +14     
+ Misses       2332     2321      -11     

Flag	Coverage Δ
multisite	70.50% <50.00%> (+0.15%)	⬆️
single	35.33% <0.00%> (+0.16%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds support/tests for handling a stray closing </p> (as produced by wpautop() with caption shortcodes expanding into FIGURE) so that computed sibling indices/XPaths remain aligned with the browser’s DOM.

Changes:

• Added a new sample document fixture covering the “stray closing </p>” scenario in HTML tag processing tests.
• Introduced an integration test-case fixture (buffer.html, expected.html, set-up.php) to validate XPath alignment and LCP-based attribute injection.
• Updated OD_HTML_Tag_Processor::next_token() to account for the implied empty P element at the stray closing </p> position by incrementing sibling indices.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
plugins/optimization-detective/tests/test-class-od-html-tag-processor.php	Adds a new data-provider case for stray closing </p> and expected breadcrumb indices.
plugins/optimization-detective/tests/test-cases/stray-closing-p/set-up.php	Registers URL metrics and a visitor to set fetchpriority based on computed XPath.
plugins/optimization-detective/tests/test-cases/stray-closing-p/expected.html	Snapshot of expected output with data-od-xpath and fetchpriority injection.
plugins/optimization-detective/tests/test-cases/stray-closing-p/buffer.html	Input buffer representing wpautop() output for the stray closing </p> scenario.
plugins/optimization-detective/class-od-html-tag-processor.php	Adjusts index bookkeeping for stray closing </p> to keep computed XPaths DOM-aligned.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[b1ink0]

LGTM