Make WordPress Core

Opened 9 years ago

Closed 5 years ago

#31703 closed defect (bug) (fixed)

Trying to get property of non-object in nav-menu.php

Reported by: mehulkaklotar's profile mehulkaklotar Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.3 Priority: normal
Severity: major Version: 4.2
Component: Menus Keywords: has-patch dev-feedback
Focuses: Cc:

Description

Every time when I refresh my site, I have this PHP error : "Trying to get property of non-object" in wp-includes/nav-menu.php in line no:665. If I have something in menu that has no post titles set for that nav object then the code should check that it is set or not.

Attachments (4)

nav-menu.diff (617 bytes) - added by mehulkaklotar 9 years ago.
checked the post_title with isset function to ensure that the post title exists if not then empty string passed which is further checked with conditions
31703.patch (1.5 KB) - added by SergeyBiryukov 9 years ago.
nav-menu.2.diff (1.3 KB) - added by mehulkaklotar 9 years ago.
nav-menu.3.diff (937 bytes) - added by mehulkaklotar 9 years ago.
code style and reformatting done

Download all attachments as: .zip

Change History (18)

@mehulkaklotar
9 years ago

checked the post_title with isset function to ensure that the post title exists if not then empty string passed which is further checked with conditions

#2 @SergeyBiryukov
9 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.2

We have validation for taxonomy menu items here, but not for post type items.

31703.patch brings some consistency.

#3 @mehulkaklotar
9 years ago

In the code below, we have $original_object->ID, in my case the ID is also not set. which causes the error.

if ( '' === $original_title ) { 
    $original_title = sprintf( __( '#%d (no title)' ), $original_object->ID ); 
} 

is should be checked first. So it is better, we check for $original_object first before doing anything.

@mehulkaklotar
9 years ago

code style and reformatting done

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


9 years ago

#5 follow-up: @DrewAPicture
9 years ago

  • Milestone changed from 4.2 to Future Release

I'd be interested to know under what circumstances the object_id wouldn't be set, thereby causing get_post() to return null. Seems like we first need to identify the cause of the reported problem before we talk about solutions.

Punting to future release.

#6 in reply to: ↑ 5 @johnjamesjacoby
9 years ago

Replying to DrewAPicture:

I'd be interested to know under what circumstances the object_id wouldn't be set, thereby causing get_post() to return null. Seems like we first need to identify the cause of the reported problem before we talk about solutions.

Punting to future release.

Without testing, it's plausible something like BuddyPress could cause this. It inserts a custom menu group for it's logged in user profile pages that are not real pages, though even in that instance it would be on us to override any values such as these if possible, and also on us to send patches upstream if not possible.

#7 @joedolson
9 years ago

I have encountered this by deleting a Page that's currently being used in a menu. But I can't get it to happen in every instance, so I'm not sure what was different about the times when I did get that result.

#8 @mehulkaklotar
9 years ago

Replying to @DrewAPicture

I'd be interested to know under what circumstances the object_id wouldn't be set, thereby causing get_post() to return null. Seems like we first need to identify the cause of the reported problem before we talk about solutions. Punting to future release.

I got the case. I have a widget in footer which has a Custom Menu widget. Now the Menu has a shopping cart page which gives a null object for get_post(). This page is from woocommerce page which was deleted but is present in the menu yet. So the object ID is being processed with get_post() where it has no object to return.

Last edited 9 years ago by mehulkaklotar (previous) (diff)

#9 @mehulkaklotar
9 years ago

  • Severity changed from normal to major

#10 @mehulkaklotar
9 years ago

  • Keywords dev-feedback added

#11 @cristiano.zanca
7 years ago

Hi, similar situation here: testing website, menu item linked to a deleted page or similar, this is the screenshot http://i.imgur.com/FuBMkwB.png

#12 @SergeyBiryukov
5 years ago

  • Milestone changed from Future Release to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#13 @SergeyBiryukov
5 years ago

#47935 was marked as a duplicate.

#14 @SergeyBiryukov
5 years ago

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

In 45891:

Menus: In wp_setup_nav_menu_item() and Walker_Nav_Menu_Edit::start_el(), check if the post or term associated with the menu item still exists to avoid a PHP notice.

If the associated post or term no longer exists, mark the menu item as invalid.

Props mehulkaklotar, kamrankhorsandi, cristiano.zanca, SergeyBiryukov.
Fixes #31703.

Note: See TracTickets for help on using tickets.