Bug: Quick Edit in User Groups PHP Error

[mikeyarce]

Expected/Desired Behavior

No PHP Errors when using Quick Edit

Actual Behavior

PHP Error:

[07-Jun-2021 17:34:31 UTC] PHP Warning:  Undefined array key "hook_suffix" in /wp-admin/includes/class-wp-screen.php on line 223

In the console:

component: "Plugin: Edit-Flow"
file: "wp-admin/includes/class-wp-screen.php"
key: "d5841370c9175d9d14dff829bd2b4b69"
line: 224
message: "Undefined array key \"hook_suffix\""
stack: Array(6)
0: "WP_Screen::get()"
1: "convert_to_screen()"
2: "WP_List_Table->__construct()"
3: "EF_Usergroups_List_Table->__construct()"
4: "EF_User_Groups->handle_ajax_inline_save_usergroup()"
5: "do_action('wp_ajax_inline_save_usergroup')"
length: 6
__proto__: Array(0)
type: "warning"
__proto__: Object

Steps to Reproduce the Problem

1. Go to Edit Flow -> User Groups
2. Click Quick Edit
3. Make a change
4. Click "Update User Group"

(Optional) Additional notes

I think the problem is here: https://github.com/Automattic/Edit-Flow/blob/master/modules/user-groups/user-groups.php#L1097-L1103

The __construct() should have screen because the method WordPress uses to determine this doesn't run when an AJAX call is made.

[htdat]

As mentioned in #666 (review), this is a bug in core WP
https://core.trac.wordpress.org/ticket/49089

[htdat]

Replicate this issue in the core

• Add this file under wp-content/mu-plugins https://gist.github.com/htdat/06d77ea326d53cb1ae1e6b68c96aa0c3
• Visit wp-admin/edit.php
• Try to edit a post with "Quick Edit" to trigger the ajax action
• Check PHP error log (debug.log)

Error with track trace:

[12-Jul-2021 04:28:47 UTC] PHP Notice: Undefined index: hook_suffix in /var/www/html/wp-admin/includes/class-wp-screen.php on line 223
[12-Jul-2021 04:28:47 UTC] PHP Stack trace:
[12-Jul-2021 04:28:47 UTC] PHP 1. {main}() /var/www/html/wp-admin/admin-ajax.php:0
[12-Jul-2021 04:28:47 UTC] PHP 2. do_action() /var/www/html/wp-admin/admin-ajax.php:187
[12-Jul-2021 04:28:47 UTC] PHP 3. WP_Hook->do_action() /var/www/html/wp-includes/plugin.php:484
[12-Jul-2021 04:28:47 UTC] PHP 4. WP_Hook->apply_filters() /var/www/html/wp-includes/class-wp-hook.php:316
[12-Jul-2021 04:28:47 UTC] PHP 5. {closure:/var/www/html/wp-content/mu-plugins/trac-49089.php:11-24}() /var/www/html/wp-includes/class-wp-hook.php:292
[12-Jul-2021 04:28:47 UTC] PHP 6. WP_List_Table->__construct() /var/www/html/wp-content/mu-plugins/trac-49089.php:14
[12-Jul-2021 04:28:47 UTC] PHP 7. convert_to_screen() /var/www/html/wp-admin/includes/class-wp-list-table.php:149
[12-Jul-2021 04:28:47 UTC] PHP 8. WP_Screen::get() /var/www/html/wp-admin/includes/template.php:2571

Root cause

The error happens around this line https://github.com/WordPress/WordPress/blob/9f91305af2bd18c914096cc5e5cc1d6882163200/wp-admin/includes/class-wp-screen.php#L223-L223

			$id = $GLOBALS['hook_suffix'];

admin-ajax.php requests do not load file wp-admin/admin.php, which is loaded when admins browse wp-admin pages.
Then global $hook_suffix is not added and defined, hence, the PHP error Undefined index: hook_suffix happens.

• https://github.com/WordPress/WordPress/blob/9f91305af2bd18c914096cc5e5cc1d6882163200/wp-admin/admin.php#L203-L210

A side note here: I searched hook_suffix and $hook_suffix across WordPress core and found out that this global $hook_suffix var is set up in file wp-admin/admin.php.

Solution

I think the ultimate solution here is to change the way we handle $GLOBALS['hook_suffix']. Here is my proposal :D

Index: wp-admin/includes/class-wp-screen.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/wp-admin/includes/class-wp-screen.php b/wp-admin/includes/class-wp-screen.php
--- a/wp-admin/includes/class-wp-screen.php	(date 1623657694433)
+++ b/wp-admin/includes/class-wp-screen.php	(date 1623657694433)
@@ -220,7 +220,10 @@
 		if ( $hook_name ) {
 			$id = $hook_name;
 		} else {
-			$id = $GLOBALS['hook_suffix'];
+			// AJAX requests does not load wp-admin/admin.php, which sets value for global $hook_suffix
+			$id = wp_doing_ajax()
+				? 'wp_ajax_' . esc_attr( $_REQUEST['action'] )
+				: $GLOBALS['hook_suffix'];
 		}
 
 		// For those pesky meta boxes.

What do you think, @mikeyarce? If you have time, I would like to see your thought too, @nielslange.

---

@mikeyarce pointed out a great point: This notice error will be converted into a warning in PHP8 and that might cause a bigger headache. Ref:

• https://stackoverflow.com/a/66125363
• https://www.php.net/manual/en/migration80.incompatible.php:

A number of notices have been converted into warnings:
...
Attempting to read an undefined array key.

[htdat]

FYI. Submitted a PR for this core issue:

• https://core.trac.wordpress.org/ticket/49089#comment:3
• WP_Screen: fix hook_suffix is undefined - trac issue 49089 WordPress/wordpress-develop#1490

[GaryJones]

This issue was fixed in commit 13621f9 (June 2021) which added set_current_screen() calls before creating WP_List_Table instances in AJAX handlers. The fix is already in the codebase and all related tests pass.