Opened 10 years ago
Last modified 3 weeks ago
#34839 new defect (bug)
Wrong attribution of current_page_parent for menus when on single/archive CPT
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.0 |
Component: | Menus | Keywords: | has-patch has-test-info needs-unit-tests |
Focuses: | ui | Cc: |
Description
- When on a single CPT page, the menu item receiving the class current_page_parent is the blog archive page, and not the archive CPT page.
- When on an archive CPT page that is in a submenu, the menu item receiving the class current_page_parent is the blog archive page, and not the parent menu item. Also, the parent menu item doesn't have the current-menu-parent and current-menu-ancestor classes.
This plugin has some functions that deal with this issue, but it's probably something that should be fixed in core now since archive CPT page are available by default. https://wordpress.org/plugins/sf-archiver/
Attachments (1)
Change History (10)
#2
in reply to:
↑ 1
@
10 years ago
Replying to helen:
Hi @tabrisrp - welcome to Trac. Is this new to trunk (4.4 development) or does it exist earlier?
Hi @helen,
Happy to join and help improving WP :)
It exists since before the 4.4, as the plugin I linked already did changes on the menu item classes and is working with 4.3 and earlier. It's probably there from the start of the new menu system.
It only came to my attention now thanks to a currently in development website.
#3
@
10 years ago
- Version changed from trunk to 3.0
Thanks - needed to check if this was something we needed to fix before releasing 4.4. Upon further inspection, I feel fairly certain this is a duplicate of something but cannot locate it at the moment.
#4
@
10 years ago
It most probably has been reported before, but I was not able to find a corresponding ticket.
It's mostly a design issue for themes, because if you're styling those classes has a position indicator on the website, it will be wrong on some pages.
I think it could get a small priority bump now that the archives CPT pages are available in the menu editor.
It's also an issue in a specific case I encountered, where I filter a menu to extract its sub-menus based on the menu parent classes. For the archives CPT page and single CPT page, the wrong sub-menu is extracted because of this bug.
This ticket was mentioned in Slack in #core by chriscct7. View the logs.
9 years ago
#6
@
9 years ago
- Keywords has-patch needs-testing added
With regard to the first issue, see #16382 which is marked "wontfix" as it seems current_page_parent
is added as a backwards-compatibility measure. New themes/plugins should best rely only on the newer classes (with hyphens).
For the other problems, code to properly set the menu item parent/ancestor classes was missing for custom post type archives. In fact, this code shouldn't be dependent on object type, so in the forthcoming patch I have factored this out into a common block. This also makes function _wp_menu_item_classes_by_context()
slightly easier to read and understand (but still pretty opaque!).
I took the opportunity of incorporating the fix for #32918 at the same time (easier now the code is in just one place).
It's likely a number of other bugs are lurking in this area of code. We could really do with some unit tests: see #32367.
This ticket was mentioned in PR #9039 on WordPress/wordpress-develop by @SirLouen.
3 weeks ago
#7
This is only a refresh of 34839.diff
Disclaimer: I have not reviewed this code. This refresh is just for testing purposes.
More code assessment is required to see if it's adequately covering the problem stated in the ticket
Trac ticket: https://core.trac.wordpress.org/ticket/34839
#8
@
3 weeks ago
- Focuses ui added; template removed
- Keywords has-test-info needs-unit-tests added; needs-testing removed
Test Report
Description
✅ This report validates whether the indicated patch works as expected.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/9039.diff
Environment
- WordPress: 6.9-alpha-60093-src
- PHP: 8.2.28
- Server: nginx/1.27.5
- Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
- Browser: Chrome 137.0.0.0
- OS: Windows 10/11
- Theme: Twenty Sixteen 3.5
- MU Plugins: None activated
- Plugins:
- Category Posts in Custom Menu - Free 3.0 3.0.3
- Test Reports 1.2.0
Testing Instructions
- I'm using
Category Posts in Custom Menu
plugin to showcase the problem easily - Add some pages. For this test, I've added 4 pages
- Add a post category
- Add 2 posts to this newly created category
- I'm using 2016 theme to configure menus more clearly
- Go to menus and add the following
- First page as parent, second page as child of first
- Third page as parent, fourth page as child of third
- Add the category as child of Third.
- Enable the feature of the plugin previously installed in the category:
Replace by posts in this category
- Go to the front end
- Click on the second page
- You will see that
current_page_ancestor
is present (check Supp artifacts) - Click on any of the two posts that popped under the Third page submenu
- 🐞 Page ancestor is not present (check Supp artifacts)
Expected Results
- Ancestors are respected in all scenarios
Actual Results
- ✅ Issue resolved with patch (check Supp Artifacts)
Additional Notes
- Now I'm noticing that #32918 is essentially the same. That ticket was before this, but given that almost a decade has passed since inception of both ticket, and this has a better patch, I'm going to close the other one in favour of this.
needs-code-review
- I have not reviewed the code, only tested the behaviour. As I commented before, further assessment is required to evaluate if this patch should move forward.
- As @mdgl commented maybe some Unit tests could be added in the same place as the ones provided in #32367 (
needs-unit-tests
)
Hi @tabrisrp - welcome to Trac. Is this new to trunk (4.4 development) or does it exist earlier?