WordPress.org

Make WordPress Core

Opened 7 weeks ago

Closed 7 weeks ago

Last modified 7 weeks ago

#54558 closed defect (bug) (fixed)

Notice: Undefined index in WordPress 5.9 Beta

Reported by: kafleg Owned by: audrasjb
Milestone: 5.9 Priority: normal
Severity: normal Version: trunk
Component: Editor Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

When we go to Appearance > Editor and then to Template Parts, I see this notice.

<b>Notice</b>: Undefined index: path in <b>E:\laragon\www\59-beta\wp-includes\rest-api.php</b> on line <b>2854</b><br />

Screenshot: https://prnt.sc/21dpjkw

Attachments (2)

54558.diff (572 bytes) - added by costdev 7 weeks ago.
Adds a leading slash to paths without one.
54558.1.diff (812 bytes) - added by costdev 7 weeks ago.
Patch updated to remove new error by covering paths of array type.

Download all attachments as: .zip

Change History (21)

#1 @costdev
7 weeks ago

Thanks for reporting this issue @kafleg!

Test Report

Env

  • WordPress 5.9-beta1-52296-src
  • Chrome 96.0.4664.45
  • Windows 10
  • Theme: Twenty Twenty Two
  • Gutenberg Editor
  • Plugin: None activated


Steps to test

  1. Navigate to Appearance > Editor.
  2. Navigate to Template Parts.

Results

The reported notice is displayed:

Notice: Undefined index: path in <path>/src/wp-includes/rest-api.php on line 2854

Notes

  • This is caused by the ?context=edit path missing a leading slash.
  • When a leading slash is added, the notice no longer appears.
Last edited 7 weeks ago by costdev (previous) (diff)

@costdev
7 weeks ago

Adds a leading slash to paths without one.

#2 @costdev
7 weeks ago

  • Keywords has-patch needs-testing added

This ticket was mentioned in Slack in #core-test by costdev. View the logs.


7 weeks ago

#4 @SergeyBiryukov
7 weeks ago

  • Component changed from General to Editor
  • Milestone changed from Awaiting Review to 5.9

@costdev
7 weeks ago

Patch updated to remove new error by covering paths of array type.

#5 @mukesh27
7 weeks ago

Hi there!

In addition to the error, I also see the below error in my error log.

PHP Warning:  array_keys() expects parameter 1 to be array, bool given in /wordpress/wp-includes/update.php on line 993
PHP Warning:  Invalid argument supplied for foreach() in /wordpress/wp-includes/update.php on line 993
Last edited 7 weeks ago by mukesh27 (previous) (diff)

This ticket was mentioned in PR #1996 on WordPress/wordpress-develop by costdev.


7 weeks ago

  • Keywords has-unit-tests added

This PR:

  • Covers $path values of type string.
  • Covers $path values of type array with values of type string.
  • Adds unit tests.

Trac ticket: https://core.trac.wordpress.org/ticket/54558

#7 follow-up: @costdev
7 weeks ago

@mukesh27 To make sure this doesn't occur at an earlier stage, can you do the following:

  1. Navigate to Appearance > Editor.
  2. Clear your debug.log file.
  3. Navigate to Template Parts.
  4. Confirm that the log now contains the error you mentioned above.
Last edited 7 weeks ago by costdev (previous) (diff)

#8 @audrasjb
7 weeks ago

  • Version set to trunk

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


7 weeks ago

#10 @audrasjb
7 weeks ago

  • Owner set to audrasjb
  • Status changed from new to reviewing

Self assigning for test/review.

#11 in reply to: ↑ 7 @mukesh27
7 weeks ago

If I follow the below steps I don't get additional errors that I post.

Replying to costdev:

  1. Navigate to Appearance > Editor.
  2. Clear your debug.log file.
  3. Navigate to Template Parts.
  4. Confirm that the log now contains the error you mentioned above.

#12 @Boniu91
7 weeks ago

Test Report

Env

  • WordPress 5.0.14 and 5.9-beta1
  • Ubuntu 20.04.2 LTS server
  • Theme: Twenty Twenty One
  • Plugin: WP Downgrade | Beta Tester

Steps to test

  1. Navigate to Appearance > Editor.
  2. Clear your debug.log file.
  3. Navigate to Template Parts.

No error in debug.log nor on the frontend.

#13 @audrasjb
7 weeks ago

  • Keywords commit added; needs-testing removed
  • Status changed from reviewing to accepted

It looks good to me as well.
As the two previous tests were successful, let's commit the last patch.

#14 @audrasjb
7 weeks ago

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

In 52312:

Editor: Avoid undefined index notices in the Template Parts Editor.

This changes adds a leading slash when needed in the ?context=edit path to avoid an undefined index notice in the Template Parts Editor.

Follow-up to [52275].

Props kafleg, costdev, mukesh27, Boniu91.
Fixes #54558.

#16 @audrasjb
7 weeks ago

In 52313:

Coding standards: Address a few coding standards issues after [52312].

Follow-up to [52312].

See #54558.

#17 @costdev
7 weeks ago

#54576 was marked as a duplicate.

#18 @TobiasBg
7 weeks ago

[52312] added a foreach loop with the value being assigned by reference.
This can lead to weird issues later on, if the variable (here: $path) is used again in the function, while the reference was not broken. While it's not in this case, let's follow best practices and add unset( $path ); after the loop.

See e.g. the example at https://www.php.net/manual/en/control-structures.foreach.php

#19 @SergeyBiryukov
7 weeks ago

In 52322:

Coding Standards: Break the $path reference after a foreach loop in block_editor_rest_api_preload().

When using a foreach loop with a value assigned by reference, the variable continues to point to the last array element even after the loop, so it is recommended to destroy it by unset() to avoid unexpected behavior later on.

See PHP Manual: foreach.

Follow-up to [52312], [52313].

Props TobiasBg.
See #54558.

Note: See TracTickets for help on using tickets.