Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#26795 closed defect (bug) (fixed)

Navigation Menu does not updates after a page or post is deleted, which leads to inconsistent menu in backend and frontend

Reported by: umeshsingla's profile UmeshSingla Owned by: ocean90's profile ocean90
Milestone: 3.9 Priority: normal
Severity: major Version: 3.7
Component: Menus Keywords: has-patch commit
Focuses: Cc:

Description

If I've added a page in menu and later I delete it permanently, It does not get remove from menu items and instead displays notice as $original_object = get_post( $menu_item->object_id ); in nav_menu.php on line 596 returns nothing.
Theme: Twentyfourteen
Wordpress Version 3.8

To replicate the issue:

  1. Create a Normal Page.
  2. Add page to menu and Update it.
  3. Delete the Page Permanently
  4. If debug is turned, php notice are there on the menu page in backend as well as frontend. If debug is off you can still see the space for items in menu.
  5. On saving the menu back, the menu item is removed.

It happens as _wp_delete_post_menu_item is not updating the menu items on delete_post hook

Attachments (7)

Menus ‹ Micro Blogging — WordPress.png (68.7 KB) - added by UmeshSingla 11 years ago.
Space for deleted pages in backend still appears.
Micro Blogging Just another WordPress site.png (77.5 KB) - added by UmeshSingla 11 years ago.
Notice on frontend menu, as the page is deleted and menu items are not yet updated.
Menus ‹ Micro Blogging — WordPress.2.png (206.3 KB) - added by UmeshSingla 11 years ago.
Notices in backend
26795.diff (934 bytes) - added by UmeshSingla 11 years ago.
Solves the issue for me
26795.nav-menu.php.patch (576 bytes) - added by UmeshSingla 11 years ago.
Updated the condition to check item type
26795.default-filters.php.patch (774 bytes) - added by UmeshSingla 11 years ago.
Update menu items on wp_trash_post filter
26795.patch (3.3 KB) - added by ocean90 11 years ago.

Download all attachments as: .zip

Change History (29)

@UmeshSingla
11 years ago

Space for deleted pages in backend still appears.

@UmeshSingla
11 years ago

Notice on frontend menu, as the page is deleted and menu items are not yet updated.

#1 @UmeshSingla
11 years ago

  • Severity changed from normal to major

#2 @nacin
11 years ago

Dropping a comment on this ticket as the new ticket notification accidentally did not go out.

@UmeshSingla
11 years ago

Solves the issue for me

#3 @UmeshSingla
11 years ago

  • Keywords has-patch needs-testing added

#4 @UmeshSingla
11 years ago

  • Keywords dev-feedback added

#5 @UmeshSingla
11 years ago

  • Keywords dev-feedback removed

#6 @UmeshSingla
11 years ago

is it going to be reviewed at all?

#7 @markoheijnen
11 years ago

I just looked at your patch to leave my first comment. I have experienced this issue before but never looked into it.

The removed code I don't know yet if that is needed but I do believe that this doesn't solve the whole problem. The problem to me is that the menu item isn't removed and I would expect that would be the case. To me that should be addressed too.

I'm curious if we should show an indication that a page/post is in a menu. Also I do want to mention that 3 days is a short period. Sometimes it takes a bit longer that someone has the time to look into it.

@UmeshSingla
11 years ago

Updated the condition to check item type

#8 @UmeshSingla
11 years ago

@markoheijnen The above patch only fixes the issue when you are permanently deleting a page and not if you just trash it.

It works absolutely fine for me, does it not fixes the issue for you if you permanently delete (and not trash) a page or post?

#9 @ocean90
11 years ago

  • Keywords needs-patch added; has-patch needs-testing removed
  • Milestone changed from Awaiting Review to 3.9

The issue is, that _wp_delete_post_menu_item() doesn't delete the nav menu object. _wp_delete_post_menu_item() is hooked into delete_post.

@UmeshSingla
11 years ago

Update menu items on wp_trash_post filter

#10 @ocean90
11 years ago

  • Keywords has-patch needs-unit-tests added; needs-patch removed

Haven't seen 26795.nav-menu.php.patch which works for me after a quick test.

The unit test test_wp_get_associated_nav_menu_items() should be extend for post types.

#11 @UmeshSingla
11 years ago

  • Keywords needs-patch added; has-patch needs-unit-tests removed

It'll require both the changes filter wp_trash_post and changes in wp_get_associated_nav_menu_items

#12 @UmeshSingla
11 years ago

  • Keywords needs-patch removed

#13 @UmeshSingla
11 years ago

  • Keywords has-patch needs-unit-tests added

#14 @ocean90
11 years ago

The menu item shouldn't be removed if a post is trashed, see #13822.

#15 @UmeshSingla
11 years ago

I just checked the ticket, there is a long debate to not remove items from menu if post/page is trashed.
So we could just go with the patch 26795.nav-menu.php.patch if it goes good with with the testing.

@ocean90
11 years ago

#16 follow-up: @ocean90
11 years ago

  • Keywords needs-unit-tests removed

26795.patch should fix this. _menu_item_object is not relevant for post types.

#17 @ocean90
11 years ago

  • Version changed from 3.8 to 3.7

Broken since [25163].

#18 @UmeshSingla
11 years ago

  • Summary changed from Deleting a page do not updates menu items and generates Notices in frontend and backend to Navigation Menu does not updates after a page or post is deleted, which leads to inconsistent menu in backend and frontend

#19 @UmeshSingla
11 years ago

  • Focuses ui accessibility added

#20 @ocean90
11 years ago

  • Focuses ui accessibility removed

There is no need for these focuses.

#21 in reply to: ↑ 16 @nacin
11 years ago

  • Keywords commit added
  • Owner set to ocean90
  • Status changed from new to assigned

Replying to ocean90:

26795.patch should fix this. _menu_item_object is not relevant for post types.

If that's right, this patch looks good to me.

#22 @ocean90
11 years ago

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

In 27150:

Nav Menu: Remove post/page items from the Nav Menu when the post/page is deleted.

This was broken through a change in [25163]. _menu_item_object in wp_get_associated_nav_menu_items() is not relevant for post types.
Adds unit tests.

props UmeshSingla for initial patch.
fixes #26795.

Note: See TracTickets for help on using tickets.