Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#31656 closed enhancement (fixed)

Add menu ID argument to `wp_page_menu()` so that fallback menus have same attributes as `wp_nav_menu()`

Reported by: lancewillett's profile lancewillett Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-patch
Focuses: Cc:


See use case in Twenty Fourteen menu behavior:

When no custom menu is set, the menu doesn't output the HTML ID attribute on the div element that wraps the pages menu.

This means things like accessibility attributes (like ARIA targets) don't work because the ID is missing from the HTML output.

Attachments (3)

31656.patch (1.8 KB) - added by lancewillett 9 years ago.
31656.2.patch (1.7 KB) - added by ocean90 9 years ago.
Without sanitize_html_class()
31656.diff (2.0 KB) - added by wonderboymusic 9 years ago.

Download all attachments as: .zip

Change History (13)

#1 @lancewillett
9 years ago

Patch also sanitizes both class and ID values before outputting them.

9 years ago

#2 @SergeyBiryukov
9 years ago

  • Milestone changed from Awaiting Review to 4.2

#3 @lancewillett
9 years ago

Please reference #31527 when this goes in.

This ticket was mentioned in Slack in #core by helen. View the logs.

9 years ago

9 years ago

Without sanitize_html_class()

#5 @ocean90
9 years ago

  • Keywords needs-refresh added
  • Milestone changed from 4.2 to Future Release
  • Type changed from defect (bug) to enhancement

I don't think that sanitize_html_class() is needed. This could lead to regressions since it's possible to define multiple CSS classes. The id attribute should be before the class attribute.
Also the current patch will print an empty id attribute by default. That should be avoided.

Punting per bug scrub.

9 years ago

#6 @wonderboymusic
9 years ago

  • Keywords needs-refresh removed
  • Milestone changed from Future Release to 4.4

#7 @wonderboymusic
9 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 34330:

Add a 'menu_id' argument to wp_page_menu() so that fallback menus have the same attributes as wp_nav_menu().

Props lancewillett, ocean90, wonderboymusic.
Fixes #31656.

#8 @obenland
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Unfortunately [34330] breaks back compat with themes that rely on the id to be missing from wp_page_menu() to style the fallback menu accurately. We should consider reverting.

#9 @lancewillett
9 years ago

I think we should keep the change — and help theme authors learn how to style the fallback in a better way, maybe with a educational post in the handbook.

#10 @obenland
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

I'm fine with more education rather than reverting and I reached out to @davidakennedy for a make/core post. Also see chat on Slack.

Note: See TracTickets for help on using tickets.