Make WordPress Core

Opened 15 years ago

Closed 15 years ago

Last modified 15 years ago

#13976 closed defect (bug) (fixed)

wp_nav_menu() uses IDs for menu-items preventing re-use on the same page

Reported by: foolsrun's profile foolsrun Owned by:
Milestone: 3.0.1 Priority: normal
Severity: normal Version: 3.0
Component: Menus Keywords: has-patch commit
Focuses: Cc:

Description

Hello,
Working to replace my hand-coded menus with the new dynamic menus in Wordpress 3.0 I've noticed a problem: menu items generated with wp nav menu() are given a unique CSS ID. This prevents re-using the same menu on the same page (my menu appears in both the header and footer) while maintaining valid HTML.

To reproduce this problem, call the a menu twice in the same theme and attempt to validate the HTML (http://validator.w3.org/).

Attachments (3)

nav-menu-item-id-filter.13976.diff (853 bytes) - added by filosofo 15 years ago.
13976.diff (1.5 KB) - added by nacin 15 years ago.
13976.2.diff (1.9 KB) - added by nacin 15 years ago.

Download all attachments as: .zip

Change History (16)

#1 @foolsrun
15 years ago

  • Keywords wp_nav_menu added

#2 follow-up: @filosofo
15 years ago

  • Keywords has-patch added

I've seen this complained about elsewhere, so we could probably use a filter on the id attribute (patch does that).

Otherwise, I don't know that we should try to ensure unique IDs in core: it's simple enough to get around the problem (make a new menu) that people using the same menu on twice on one page can add a couple lines to their theme's functions.php file if they really want to do that.

#3 in reply to: ↑ 2 @foolsrun
15 years ago

Am I understanding correctly that this would prevent ANY unique ID or class from being applied?

While I suppose that does solve the problem, it removes functionality afforded by having a unique element to each menu item. It seems to me that using classes rather than IDs would solve this problem while maintaining a unique identifier for each item.

Maintaining multiple menus with the same items in them seems like it defeats the purpose of having the menu defined in one place.

#4 @filosofo
15 years ago

What I'm saying is that this scenario--printing the same menu twice on one page--is unusual enough that we shouldn't try to handle it in core, but the attached patch makes it so someone who wants to can make unique IDs in just a few lines of code.

I concede that it might be better just to get rid of the id attribute altogether, but there's precedent for including them in the page- and category-listing functions.

If you really don't like it currently, pass your own Walker class to wp_nav_menu, or use regex to filter out the id attributes at one of the several filters the markup goes through before being printed.

#5 @foolsrun
15 years ago

What about allowing an arbitrary prefix to the IDs?
Instead of

<li id="menu-item-123">

I could define in the wp_nav_menu() arguments that I want them to read

<li id="header-menu-item-123">

#6 @shidouhikari
15 years ago

what about not using ID at all? Use only classes, and if consumer wants an ID he puts hte menu inside a div or span.

#7 @filosofo
15 years ago

shidouhikari,

As I mentioned in my previous comment, there's precedent in the page- and category-listing functions, and for many people apparently it's important that wp_nav_menu be similar to them in output.

#8 @shidouhikari
15 years ago

I see... but I still think it's easier and simpler to convert ids to classes other than parsing or using regex to fix. Wordpress have beeing losing quality in its standards and passing features for plugins to handle, which should be kept as standard defined by core.

We shouldn't receive poorly standardized stuff from core and spend precious CPU time to fix them, making results different from the standard.

Also, *in Wordpress* menus were always neglected and less used. Joomla themes for exemple use to have the same menu on top, left, right, etc.

There's no real need to use id when we have class, so why insist to use it? Why force users to be limited to use each menu only once per page, or force them to use tricky techniques to make it possible, if those thechies shouldn't be needed at all? It becomes more complex than needed, and throws the responsibility that should be from core to plugins, making it worse and worse.

#9 @nacin
15 years ago

  • Keywords menu id css wp_nav_menu removed

I agree with shidouhikari. We allow the same menu to be used twice. For example, in the header and in the footer. My suggestion -- make it a class, but leave the ID for first usage.

#10 @shidouhikari
15 years ago

Trying to make me cleaner.

Core devs shouldn't suggest what WP users should and shouldn't do. I can think it's not common, usual, recommended, etc enough to use the same menu more than once per page, but a site may require that feature, therefore I can't block a designer from doing it just because I don't need, I don't want or I don't like it.

If I can me it easy (not use blocking ID attribute at all and use only class attribute, or even better develop options to allow full customization of menus UL, LI, A, etc), why make it hard?

A use case is Joomla, where the same menu is used on 2 or even 4 places on the theme, with different styles. Wordpress just never used many menus because we didn't and don't have good menu generators, we barely could control pages order, and it's hard for themes to rely on plugins with proper menu generators.

The ideal solution would be having a UI to create a menu with its itens, including nested ones, having standard classes to point current page, post type, etc and their children, and allow us to set custom classes as we do with body_class, post_class, comment_class. Then the function to print the menu has a parameter to set its ID.

@nacin
15 years ago

@nacin
15 years ago

#11 @nacin
15 years ago

  • Keywords commit added

Attached patch (13976.2.diff) uses filosofo's filter, and latches on a callback to then ensure we're only using the id menu-item-* once.

#12 @nacin
15 years ago

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

(In [15407]) Prevent the same menu item from receiving duplicate IDs if the menu is used twice. All menu items now get a class with their post ID; only the first item now gets an ID. fixes #13976 for trunk.

#13 @nacin
15 years ago

(In [15408]) Prevent the same menu item from receiving duplicate IDs if the menu is used twice. All menu items now get a class with their post ID; only the first item now gets an ID. fixes #13976 for 3.0.

Note: See TracTickets for help on using tickets.