Skip to content

Add missing tags to openImpliesClose for open <p> elements - #2493

Open
FirmaSpring wants to merge 1 commit into
fb55:masterfrom
FirmaSpring:fix/p-implied-close-hgroup-menu-search
Open

Add missing tags to openImpliesClose for open <p> elements#2493
FirmaSpring wants to merge 1 commit into
fb55:masterfrom
FirmaSpring:fix/p-implied-close-hgroup-menu-search

Conversation

@FirmaSpring

@FirmaSpring FirmaSpring commented Aug 16, 2026

Copy link
Copy Markdown

Problem

Per the WHATWG HTML parsing spec (https://html.spec.whatwg.org/multipage/syntax.html#optional-tags), the list of start tags whose start implies the end of an open <p> element includes center, dialog, dir, hgroup, menu, search and summary. These entries are missing from the openImpliesClose map in src/Parser.ts, so htmlparser2 nests these elements inside an open <p> instead of closing it, diverging from browser behavior. For example, parsing <p>text<menu>item</menu> keeps the <menu> inside the paragraph.

Fix

Add the seven missing entries to openImpliesClose:

  • center, dialog, dir, hgroup, menu, search, summary -> close open <p>

Verification

  • Added a regression test in src/Parser.spec.ts asserting that an open <p> is implicitly closed before <hgroup> opens.
  • vitest run: 7 test files, 188 tests passed (including the new regression test).
  • Existing fixtures/snapshots contain no occurrences of the affected tags, so no snapshots needed updating.

Co-authored-by: FirmamentalSpring 287222957+FirmaSpring@users.noreply.github.com

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved HTML parsing for additional elements that implicitly close open paragraphs.
    • Added support for correct paragraph handling around elements such as headings groups, menus, dialogs, and summaries.
  • Tests

    • Added coverage verifying that an open paragraph closes before an <hgroup> element begins.

Per the WHATWG HTML parsing spec, the list of start tags whose start
implies the end of an open <p> element includes center, dialog, dir,
hgroup, menu, search and summary. These entries were missing from the
openImpliesClose map, so htmlparser2 nested these elements inside an
open <p> instead of closing it, diverging from browser behavior.

Add the seven missing entries and a regression test asserting that an
open <p> is closed before <hgroup> opens.

All 188 tests pass (vitest run).

Co-authored-by: FirmamentalSpring <287222957+FirmaSpring@users.noreply.github.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7a1018fb-7f09-4ae9-a1d8-9b733a2473df

📥 Commits

Reviewing files that changed from the base of the PR and between 34ec526 and 6936c88.

📒 Files selected for processing (2)
  • src/Parser.spec.ts
  • src/Parser.ts

Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The parser now implicitly closes an open <p> before center, dialog, dir, hgroup, menu, search, and summary. A test verifies the close event precedes the <hgroup> opening event.

Changes

HTML implicit close handling

Layer / File(s) Summary
Paragraph close mappings and coverage
src/Parser.ts, src/Parser.spec.ts
The HTML tag map adds seven paragraph-closing elements. Tests verify automatic <p> closure before <hgroup> starts.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 6936c

This localized parser change adds the missing implicit paragraph-closing behavior and includes regression coverage; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested reviewers: vimzh

Poem

A rabbit checks the tags in line,
“Close the paragraph; all is fine.”
<hgroup> starts with events in place,
Seven tags now share the case.
Hop, hop—clean parsing wins!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding missing tags to openImpliesClose so they close open

elements.

Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 16, 2026

Copy link
Copy Markdown

Greptile Summary

This PR aligns implicit paragraph closing with the WHATWG optional-tag rules.

  • Adds center, dialog, dir, hgroup, menu, search, and summary to the start tags that close an open <p>.
  • Adds a regression test verifying that <hgroup> closes <p> before its open callback.

Confidence Score: 5/5

The PR appears safe to merge, with the parser additions matching the existing implicit-close mechanism and no actionable regressions identified.

The new tags reuse the established HTML-mode paragraph-closing behavior, and the regression test verifies the expected implied-close callback and ordering.

Important Files Changed

Filename Overview
src/Parser.ts Adds seven standards-aligned entries to the existing HTML-mode implicit-close map without introducing a new parsing path.
src/Parser.spec.ts Adds a focused callback-order regression test for implicit paragraph closure before <hgroup>.

Reviews (1): Last reviewed commit: "Add missing tags to openImpliesClose for..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant