Opened 3 years ago

Closed 3 years ago

#12959 closed defect (bug) (fixed)

New menu systems unrecoverable error

Reported by: Dickie Owned by: filosofo
Priority: high Milestone: 3.0
Component: Menus Version: 3.0
Severity: major Keywords: menu has-patch
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 3 years ago.
12959.diff (2.3 KB) - added by nacin 3 years ago.

Download all attachments as: .zip

Change History (14)

Replicatable steps...

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

Bang

  • 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.

  • 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.

nacin3 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.

  • Owner changed from ryan to filosofo
  • Status changed from new to assigned
  • Status changed from assigned to accepted
  • Resolution set to fixed
  • Status changed from accepted to closed

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

Tested, and works for me. Fixed.

comment:9 follow-up: ↓ 11   pearsonified3 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.

  • Version changed from 3.0.1 to 3.0

Version should track the original bug report.

comment:11 in reply to: ↑ 9   nacin3 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.

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

No response, re-closing as fixed.

Note: See TracTickets for help on using tickets.