Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#54558 closed defect (bug) (fixed)

Notice: Undefined index in WordPress 5.9 Beta

Reported by: kafleg's profile kafleg Owned by: audrasjb's profile audrasjb
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.9
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 3 years ago.
Adds a leading slash to paths without one.
54558.1.diff (812 bytes) - added by costdev 3 years ago.
Patch updated to remove new error by covering paths of array type.

Download all attachments as: .zip

Change History (21)

#1 @costdev
3 years 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 > Site 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.
Version 0, edited 3 years ago by costdev (next)

@costdev
3 years ago

Adds a leading slash to paths without one.

#2 @costdev
3 years ago

  • Keywords has-patch needs-testing added

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


3 years ago

#4 @SergeyBiryukov
3 years ago

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

@costdev
3 years ago

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

#5 @mukesh27
3 years 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 3 years ago by mukesh27 (previous) (diff)

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


3 years ago
#6

  • 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
3 years 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 3 years ago by costdev (previous) (diff)

#8 @audrasjb
3 years ago

  • Version set to trunk

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


3 years ago

#10 @audrasjb
3 years ago

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

Self assigning for test/review.

#11 in reply to: ↑ 7 @mukesh27
3 years 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
3 years 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
3 years 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
3 years 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
3 years ago

In 52313:

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

Follow-up to [52312].

See #54558.

#17 @costdev
3 years ago

#54576 was marked as a duplicate.

#18 @TobiasBg
3 years 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
3 years 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.