Make WordPress Core

Opened 12 years ago

Closed 11 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 Owned by: wonderboymusic
Priority: normal Milestone: 4.4
Component: Menus Version:
Severity: normal Keywords: has-patch
Cc: Focuses:

Description

See use case in Twenty Fourteen menu behavior: https://core.trac.wordpress.org/ticket/31527#comment:25

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 12 years ago.
31656.2.patch​ (1.7 KB ) - added by ocean90 12 years ago.
Without sanitize_html_class()
31656.diff​ (2.0 KB ) - added by wonderboymusic 11 years ago.

Download all attachments as: .zip

Change History (13)

#1 @lancewillett
12 years ago

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

@lancewillett
12 years ago

#2 @SergeyBiryukov
12 years ago

  • Milestone Awaiting Review → 4.2

#3 @lancewillett
12 years ago

Please reference #31527 when this goes in.

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


12 years ago

@ocean90
12 years ago

Without sanitize_html_class()

#5 @ocean90
12 years ago

  • Keywords needs-refresh added
  • Milestone 4.2 → Future Release
  • Type defect (bug) → 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.

#6 @wonderboymusic
11 years ago

  • Keywords needs-refresh removed
  • Milestone Future Release → 4.4

#7 @wonderboymusic
11 years ago

  • Owner set to wonderboymusic
  • Resolution → fixed
  • Status new → 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
11 years ago

  • Resolution fixed
  • Status closed → 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
11 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
11 years ago

  • Resolution → fixed
  • Status reopened → 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.