WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#12959 closed defect (bug) (fixed)

New menu systems unrecoverable error

Reported by: Dickie Owned by: filosofo
Milestone: 3.0 Priority: high
Severity: major Version: 3.0
Component: Menus Keywords: menu has-patch
Focuses: Cc:

Description

I had one of the new menu system set up that included a set of categories.
I then decided to delete some of the categories (in the category menu) that were still included in the menu, and now get the following error message on the menu admin screen, (and the front end as well) with no option to delete the offending menu or create a new one.

Catchable fatal error: 
Object of class WP_Error could not be converted to string in
/home/*******/public_html/wp-includes/formatting.php on line 433

I tried re-adding the categories that I deleted, but to no avail. It looks like I may need to delve into the database to fix this one.

Attachments (2)

nav_menu_items_auto_delete.diff (1.9 KB) - added by ptahdunbar 9 years ago.
12959.diff (2.3 KB) - added by nacin 9 years ago.

Download all attachments as: .zip

Change History (14)

#1 @Dickie
9 years ago

Replicatable steps...

Create a category... add it to a new menu...
Delete the category.
Go back to menu editor.

Bang

#2 @ptahdunbar
9 years ago

  • Keywords has-patch commit added

nav_menu_items_auto_delete.diff fixes this bug by deleting any instances of an object (category, page, post, etc.) from all nav menus when they are removed from the db.

#3 @nacin
9 years ago

  • Keywords commit removed
  • Milestone changed from Unassigned to 3.0

If a post gets deleted and a menu item is a term with the same ID as the post, it accidentally gets the axe.

We don't account for trashed posts. I've additionally moved the actions to the post-delete actions (deleted_post versus delete_post). And we need to call wp_delete_post with a $force_delete arg.

Wouldn't this cause a potential loop? i.e. if the function needs to delete a nav_menu_item post, then it'll fire all the hooks all over again for that. We need a way to stop that.

Posting my patch so far.

@nacin
9 years ago

#4 @nacin
9 years ago

Aside from that, I forgot to mention one of the more important notes. In this patch, we would be looping through every menu item of every menu and pulling up to two post meta values before actually deleting anything. I think this could justify direct queries.

#5 @nacin
9 years ago

  • Owner changed from ryan to filosofo
  • Status changed from new to assigned

#6 @filosofo
9 years ago

  • Status changed from assigned to accepted

#7 @nacin
9 years ago

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

I think this one was handled elsewhere. Re-open with a new report.

#8 @koopersmith
9 years ago

Tested, and works for me. Fixed.

#9 follow-up: @pearsonified
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from 3.0 to 3.0.1

I just tested this with custom taxonomies on 3.0.1, and it failed. It looks as though categories and tags were fixed, but a slight adjustment will be necessary to accommodate the deletion of taxonomies.

#10 @mrmist
9 years ago

  • Version changed from 3.0.1 to 3.0

Version should track the original bug report.

#11 in reply to: ↑ 9 @nacin
9 years ago

Replying to pearsonified:

I just tested this with custom taxonomies on 3.0.1, and it failed. It looks as though categories and tags were fixed, but a slight adjustment will be necessary to accommodate the deletion of taxonomies.

Do you mean the removal of terms in custom taxonomies, or the unregistration of a taxonomy? The latter is a separate bug report, #13958.

#12 @nacin
9 years ago

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

No response, re-closing as fixed.

Note: See TracTickets for help on using tickets.