WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 2 months ago

#39564 closed defect (bug) (fixed)

Notice: Undefined index: item_spacing in /var/www/html/wordpress/wp-includes/class-walker-page.php on line 106

Reported by: jrswgtr Owned by: dd32
Milestone: 4.7.3 Priority: normal
Severity: normal Version: 4.7
Component: Menus Keywords: has-patch commit fixed-major
Focuses: Cc:

Description

I get this notice while trying to generate a sitemap with the page walker.

Notice: Undefined index: item_spacing in /var/www/html/wordpress/wp-includes/class-walker-page.php on line 106

Attachments (3)

39564.patch (1.5 KB) - added by bhargavbhandari90 4 months ago.
39564.2.patch (1.5 KB) - added by ketuchetan 3 months ago.
Added patch from the src directory
39564.3.patch (1.7 KB) - added by ketuchetan 2 months ago.
Changed the condition based on the @dd32 suggestion.

Download all attachments as: .zip

Change History (21)

#1 @SergeyBiryukov
4 months ago

  • Component changed from General to Menus
  • Milestone changed from Awaiting Review to 4.7.2

Introduced in [38523].

#2 @ketuchetan
4 months ago

Actually, I am no able to reproduce this issue. @jrswgtr please provide me the steps to reproduce the issue.

#3 @peterwilsoncc
4 months ago

  • Owner set to peterwilsoncc
  • Status changed from new to accepted

#4 @bhargavbhandari90
4 months ago

Thanks @jrswgtr for reporting this.
I am able to replicate this notice at lines 106, 56, 199 and 81.

Last edited 4 months ago by bhargavbhandari90 (previous) (diff)

#5 @bhargavbhandari90
4 months ago

  • Keywords has-patch needs-testing added

@ketuchetan Try by adding this code into your functions.php. Make sure you have made Debugging True from your wp-config.php. You can also check by any debugging plugin.

function test_page_walker() {
	$walker_page = new Walker_Page();
	echo '<ul>'.$walker_page->walk(get_pages(), 0).'</ul>'; // 0 means display all levels.	
}
add_action( 'init', 'test_page_walker' );
Last edited 4 months ago by bhargavbhandari90 (previous) (diff)

@ketuchetan
3 months ago

Added patch from the src directory

#6 @bhargavbhandari90
3 months ago

Hi, @ketuchetan

I saw that your patch is duplicate. Can you explain why? :-) I think you should have a better solution than the previous patch. :-)

#7 follow-up: @ketuchetan
3 months ago

@bhargavbhandari90 There is no any changes but you can see my patch I have created patch from the /src/ directory of development repository.

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


3 months ago

#9 in reply to: ↑ 7 @peterwilsoncc
3 months ago

Replying to ketuchetan:

@bhargavbhandari90 There is no any changes but you can see my patch I have created patch from the /src/ directory of development repository.

Thanks, I appreciate the effort. The grunt patch command is really good at picking up sub-directories and will prompt if needs be. While helpful, refreshes like these are unnecessary.

#10 follow-up: @peterwilsoncc
3 months ago

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

In 39949:

Menus: Prevent notice thrown in class-walker-page.php.

Calling Walker_Page::walk() directly was causing an Undefined index: item_spacing notice to be thrown, this adds an isset() check to prevent it.

Props bhargavbhandari90.
Fixes #39564.

#11 @peterwilsoncc
3 months ago

  • Keywords commit fixed-major added; needs-testing removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to 4.7 branch.

#12 @peterwilsoncc
3 months ago

  • Owner changed from peterwilsoncc to dd32
  • Status changed from reopened to assigned

#13 in reply to: ↑ 10 @bhargavbhandari90
3 months ago

Replying to peterwilsoncc:

In 39949:

Menus: Prevent notice thrown in class-walker-page.php.

Calling Walker_Page::walk() directly was causing an Undefined index: item_spacing notice to be thrown, this adds an isset() check to prevent it.

Props bhargavbhandari90.
Fixes #39564.

Thank you @peterwilsoncc

Last edited 3 months ago by bhargavbhandari90 (previous) (diff)

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


3 months ago

#15 follow-up: @dd32
2 months ago

@peterwilsoncc in [39949] I note that the logic has now changed from the 4.6 behaviour in the event that the item_spacing arg is not provided - it'll default to discard in that case now.

Should this have been if ( ! isset(...) || 'preserve' ...?

@ketuchetan
2 months ago

Changed the condition based on the @dd32 suggestion.

#16 in reply to: ↑ 15 @peterwilsoncc
2 months ago

Replying to dd32:

@peterwilsoncc in [39949] I note that the logic has now changed from the 4.6 behaviour in the event that the item_spacing arg is not provided - it'll default to discard in that case now.

Should this have been if ( ! isset(...) || 'preserve' ...?

That's a good catch.

It looks like the behaviour has changed when the walker is called directly rather than via wp_list_pages() and related functions. I'm inclined to leave it as it's an edge case and, arguably, ill advised.

#17 @dd32
2 months ago

It looks like the behaviour has changed when the walker is called directly rather than via wp_list_pages() and related functions. I'm inclined to leave it as it's an edge case and, arguably, ill advised.

I'm inclined to agree, the only thing that I can think of is the case where you've extended the Walker with a custom class. However, in that case you should still be using wp_list_pages() and passing the walker arg.
I'm fine with leaving this as-is then.

#18 @dd32
2 months ago

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

In 40092:

Menus: Prevent notice thrown in class-walker-page.php.

Calling Walker_Page::walk() directly was causing an Undefined index: item_spacing notice to be thrown, this adds an isset() check to prevent it.

Props bhargavbhandari90, peterwilsoncc.
Merges [39949] to the 4.7 branch.
Fixes #39564.

Note: See TracTickets for help on using tickets.