Navigation: Don't repeat the block anchor id on the inner list - #80975
Navigation: Don't repeat the block anchor id on the inner list#80975wpgaurav wants to merge 2 commits into
Conversation
The navigation block renders its inner `<ul>` with `get_block_wrapper_attributes()` so that Core's layout and alignment styles, whose selectors target the list rather than the outer `<nav>`, keep applying. That also copies the `anchor` block support's `id` onto the list, so a custom HTML Anchor is emitted twice and the markup contains duplicate ids. Strip the `id` from the list's opening tag with `WP_HTML_Tag_Processor`, leaving the anchor on the `<nav>` wrapper alone. The repeated classes described in WordPress#62690 are deliberately left untouched here: removing them risks breaking theme styles and needs a dev note, whereas duplicate ids are unambiguously invalid HTML and can be fixed on their own. Fixes WordPress#80331 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
github-actions
Bot
added
[Package] Block library
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @wpgaurav! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
1 participant
What?
Closes #80331
Stops the Navigation block from rendering a custom HTML Anchor on both the outer
<nav>and the inner<ul>, which produced duplicateidattributes.Why?
Setting Block Settings → Advanced → HTML Anchor on a Navigation block emits the same
idtwice:IDs must be unique in a document, so this is invalid HTML. It affects CSS selectors, JavaScript lookups, fragment links, and HTML/accessibility validators.
The root cause is that
get_inner_blocks_html()callsget_block_wrapper_attributes()a second time for the list element, which re-applies everything theanchorblock support contributed to the wrapper.This is the same underlying call flagged in #62690, but that issue is about the repeated class. The two are worth separating:
<ul>rather than the<nav>, so the list genuinely relies on those wrapper attributes. Changing it can break theme styles and would need a dev note, and existing e2e tests assert the class is present.idhas none of those properties. It is unambiguously invalid, nothing depends on it, and no test asserts it.So this PR fixes only the
idand deliberately leaves the class duplication to #62690 / #63801.How?
get_inner_blocks_html()now builds the list's opening tag through a smallget_list_open_tag()helper, which removes theidwithWP_HTML_Tag_Processorand leaves every other wrapper attribute untouched. The anchor remains on the<nav>.WP_HTML_Tag_Processoris used rather than a regular expression, per the concern raised on #63801 about regex-based attribute rewriting.The helper carries a docblock explaining why the list is rendered with wrapper attributes at all — #62690 notes that this has never been documented, and it has caused repeated confusion.
Note for coordination: #63801 is open against the class half of this and edits the same function, so the two will need a trivial rebase depending on merge order.
Testing Instructions
Using a block theme, add a Navigation block to a post or page with a few links.
Open Block Settings → Advanced → HTML Anchor and enter
header-navigation.Save and view the post on the front end.
View source, or run this in the browser console:
Expected:
1, and theidis on the outer<nav>. On trunk this returns2.Confirm the menu still renders and is styled as before, and that
#header-navigationstill works as a fragment link target.Also worth checking that a Navigation block with no HTML Anchor is unchanged.
Testing Instructions for Keyboard
No UI or interaction changes — this only affects rendered front-end markup, so there is no keyboard-specific behaviour to test. Keyboard navigation through the menu links is unaffected.
Screenshots or screencast
document.querySelectorAll('#header-navigation').length === 2— the anchor matches both the<nav>and the inner<ul>, so a single#header-navigationCSS rule paints two nested outlines.… === 1— the anchor matches only the<nav>wrapper, and the same rule paints one outline.Use of AI Tools
This pull request was authored with AI assistance (Claude, via Claude Code), and I have reviewed and take responsibility for its contents.
Specifically: the AI was used to locate the root cause, write the change and its docblock, draft the PHPUnit tests, and draft this description. I directed the work, reviewed the diff, and verified the behaviour myself.
One limitation to flag for reviewers: the PHPUnit suite could not be executed locally, as this environment has no Docker/
wp-env. The new tests' assertions were instead verified by running them against the built class through WP-CLI on a local WordPress install, and the rendered-markup change was confirmed end to end on that site. CI will be the first true PHPUnit run.PHPCSpasses cleanly on both changed files.