Make WordPress Core

Opened 19 months ago

Closed 19 months ago

Last modified 19 months ago

#56574 closed defect (bug) (fixed)

Docs: Wrong parameter type used for Walker_Nav_Menu class

Reported by: dilipbheda's profile dilipbheda Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-patch commit good-first-bug
Focuses: docs Cc:

Description

In class-walker-nav-menu.php nav_menu_item_id filter inline document define wrong @param type.
$menu_id @param should be int instead of string

Change History (8)

mukeshpanchal27 commented on PR #3256:


19 months ago
#2

Great catch @dilipbheda

#3 @mukesh27
19 months ago

  • Keywords commit good-first-bug added
  • Milestone changed from Awaiting Review to 6.1

Hi there!

Thanks for the ticket and patch.

$menu_item->ID is always integer value so this changes make sense. The PR is LGTM and approved.

Added ticket in 6.1 milestone for visibility.

#4 @audrasjb
19 months ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

#5 @robinwpdeveloper
19 months ago

My conclusion is existing code is alright!

Here all 4 param descriptions are for the filter.
First value of the filter is

'menu-item-' . $menu_item->ID

So, $menu_id is passed as string to the filter.

I haven’t tested the code by using the filter though.

Please correct me if I am wrong.

#6 @dilipbheda
19 months ago

  • Resolution set to invalid
  • Status changed from reviewing to closed

@robinwpdeveloper Yes, you are right.

honestly, when I created a ticket, I failed to see the first argument properly.

I'm going to close this ticket.

Thank you all

#7 @SergeyBiryukov
19 months ago

In 54178:

Docs: Clarify documentation for the nav_menu_item_id filter.

This aims to make it clear that the filter is applied to an HTML ID attribute for the menu item's <li> element, and not a numeric ID.

Follow-up to [15407], [25410], [27201], [38559], [51739].

Props dilipbheda, robinwpdeveloper, mukesh27, audrasjb, SergeyBiryukov.
See #56574, #55646.

#8 @SergeyBiryukov
19 months ago

  • Resolution changed from invalid to fixed

Hi there, thanks for the ticket!

Even if the previous documentation was technically correct, it looks like it could use some clarification.

[54178] should hopefully help avoid any further confusion :)

Note: See TracTickets for help on using tickets.