WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 3 months ago

#17077 reopened defect (bug)

Page parent li elements in nav menus not always given current-page-parent class

Reported by: andymacb Owned by:
Milestone: Awaiting Review Priority: normal
Severity: minor Version: 3.1
Component: Menus Keywords: needs-patch, bulk-reopened
Focuses: Cc:

Description

Using a fresh WP3.1.1, 2010 theme, Theme Unit Test data, adding a navigation menu with Parent Page as an item without any child items and then navigating in the browser to Child Page 1.

The <li> for the parent menu item doesn't get assigned the current-page-parent class.

It does get the current-page-ancestor class, and none of my themes demand the current-page-parent class specifically so I've tagged this as trivial.

Attachments (1)

17077.patch (1.2 KB) - added by codeclarified 6 years ago.
Checks if $queried_object parent matches $parent_item ID

Download all attachments as: .zip

Change History (15)

#1 @andymacb
8 years ago

  • Cc am@… added

@codeclarified
6 years ago

Checks if $queried_object parent matches $parent_item ID

#2 @codeclarified
6 years ago

Patch adds check inside _wp_menu_item_classes_by_context() to see if $queried_object->post_parent matches $parent_item->object_id

Also updated the conditional check for applying the classes for backward compatibility. Instead of checking for current-menu-* now checking for current-page-*, seems more correct, however I'm not sure if the -menu- classes were selected for a legitimate reason

#3 @codeclarified
6 years ago

  • Keywords has-patch added

#4 @ruud@…
6 years ago

Hi, I tested the patch in version WordPress 3.7-beta1-25639-src, here are my results:

(only showing relevant classes)

parent page of selected child page:
before patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor
after patch: current-menu-ancestor current-menu-parent current_page_parent current-page-parent

parent page of selected child page after making it is actual parent page (in hierarchy):
before patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor current-page-ancestor
after patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor current-page-ancestor current-page-parent

parent page of selected child page after making it is actual parent page (in hierarchy sense) and after re-saving menu:
before patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor current-page-ancestor current-page-parent
after patch: current-menu-ancestor current-menu-parent current_page_parent current_page_ancestor current-page-ancestor current-page-parent

As far as I can see this patch fixes the problem that the menu had to get saved again to add the 'current-page-parent' class.

However 2 more things are not right in my opinion for the situation where the parent page when their only relation is in the menu:
1) having the 'current-page-parent' class in the li for a page which is acually no parent of any other page seems not appropriate
2) the code in the patch doesn't look for further ancestry.

Thanks!

#5 @ruud@…
6 years ago

  • Cc ruud@… added
  • Component changed from General to Menus
  • Keywords needs-patch added; has-patch removed
  • Severity changed from trivial to minor

OK, I looked into this way further together with @DH-Shredder and @Obenland today at the WCEU contributor day and this is what we found:

  • The initial problem is an effect of the menu not getting updated when a posttype relation is set or un-set.

This latter point is relevant because with this patch only the adding of a posttype relation is represented OK in the menu, but un-setting a relation still reports a parent relation in the menu after un-setting it (if that makes sense).

We concluded that getting rid of this bug, we should try to update any relevant menu's after setting or un-setting a posttype relation.

The second half of the patch (setting the backwards compatible classes based on page-parent instead of menu-parent) is probabbly not a good solution, because the former underscore classes don't reflect the posttype relation, but the menu relation... so checking the menu relation makes sense.

Thanks!

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


4 years ago

#7 @jorbin
4 years ago

  • Milestone changed from Awaiting Review to 4.3

The description above still seems accurate. Our docs say that:

.current-{object}-parent
This class is added to menu items that correspond to the hierachical parent of the currently rendered object, where {object} corresponds to the the value used for .menu-item-object-{object}

We should follow our docs.

#8 @obenland
4 years ago

  • Owner set to jorbin
  • Status changed from new to assigned

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


4 years ago

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


4 years ago

#11 @jorbin
4 years ago

  • Milestone changed from 4.3 to Future Release

Punting due to lack of activity. This needs a patch.

#12 @jorbin
4 years ago

  • Owner jorbin deleted

#13 @iseulde
5 months ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

This ticket has not seen any activity in over *two* years, so I'm closing it as "wontfix".

The ticket may lack decisiveness, may have become irrelevant, or may not have gathered enough interest.

If you think this ticket does deserve some attention again, feel free to reopen.

For bugs, it would be great if you could provide updated steps to reproduce against the latest version of WordPress (5.0.2 at the time of writing). Remember images or a video can be superior to explain a problem. At the very least, quickly test again to make sure the bug still exists.

If it’s an enhancement or feature, some extra motivation may help.

Thank you for your contributions to WordPress! <3

#14 @JeffPaul
3 months ago

  • Keywords bulk-reopened added
  • Milestone set to Awaiting Review
  • Resolution wontfix deleted
  • Status changed from closed to reopened

A decision was made to reopen tickets that were closed in the bulk edit that this ticket was affected by. This ticket is being placed back into the Awaiting Review milestone so it can be individually evaluated and verified to determine if it is still relevant/valid or reproducible.

Note: See TracTickets for help on using tickets.