Admin functions throw errors on widgets screen.

[peterwilsoncc]

Description

Using an admin only function, such as is_plugin_available(), in a legacy widget's form throws a 500 error on the new widget editing screen.

The Compact Archives plugin I maintain for work includes a call to is_plugin_available() in the My_Widget_Class::form() function. In previous version of WP this was dandy but in 5.8 it triggers a 500 error as the function isn't available via the REST API. I imagine the same problem applies for many other admin only functions.

Step-by-step reproduction instructions

1. Install and activate Classic Widget plugin
2. Install and activate plugin using an admin function in its widget code (eg, Compact Archives 4.0.0 (zip))
3. Add the widget on the classic widget screen. Everything will work as expected 🌻
4. Deactivate the Classic Widget plugin
5. Add the widget on the new widget screen. An error alert will show in the browser and a fatal in the PHP logs

Expected behaviour

No fatal error (possibly by including the admin function in certain situations).

Actual behaviour

Previously compatible widgets avoid throwing fatals

Screenshots or screen recording (optional)

Code snippet (optional)

Stack trace of fatal error:

PHP Fatal error:  Uncaught Error: Call to undefined function is_plugin_active() in /vagrant/content/plugins/compact-archives/inc/class-wpbeginner-caw-widget.php:103
Stack trace:
#0 /vagrant/wp/wp-includes/rest-api/endpoints/class-wp-rest-widget-types-controller.php(545): WPBeginner_CAW_Widget->form(Array)
#1 /vagrant/wp/wp-includes/rest-api/endpoints/class-wp-rest-widget-types-controller.php(484): WP_REST_Widget_Types_Controller->get_widget_form(Object(WPBeginner_CAW_Widget), Array)
#2 /vagrant/wp/wp-includes/rest-api/class-wp-rest-server.php(1140): WP_REST_Widget_Types_Controller->encode_form_data(Object(WP_REST_Request))
#3 /vagrant/wp/wp-includes/rest-api/class-wp-rest-server.php(987): WP_REST_Server->respond_to_request(Object(WP_REST_Request), '/wp/v2/widget-t...', Array, NULL)
#4 /vagrant/wp/wp-includes/rest-api/class-wp-rest-server.php(414): WP_REST_Server->dispatch(Object(WP_REST_Request))
#5 /vagrant/wp/wp-includes/rest-api.php(370): WP_REST_Server->serve_request('/wp/v2/widget-t...')
#6 /vagrant/wp/wp-includes/class-w in /vagrant/content/plugins/compact-archives/inc/class-wpbeginner-caw-widget.php on line 103

WordPress information

• WordPress version: 5.8 RC3
• Gutenberg version: Not Installed
• Are all plugins except Gutenberg deactivated? No, testing plugins for compatibility
• Are you using a default theme (e.g. Twenty Twenty-One)? Yes, Twenty Twenty-One.

Device information

• Device: Laptop
• Operating system: macOS Big Sur
• Browser: Firefox Developer Edition 91.0b1

[TimothyBJacobs]

The fix here would be to bootstrap the whole admin during this request, which I don't really love. A fatal is pretty bad though.

[adamziel]

To use these functions we need to load them, so the only debatable point here is whether we should load them eagerly or lazily. I would love to go for the lazy loading, but unfortunately there's a catch.

If we were talking about classes, we could just add an autoloader via spl_autoload_register and call it a day. Unfortunately, PHP does not support autoloading functions so no dice here.

The only other way I can think of is having an auto-generated file with "proxy functions" that would only load the admin when that's necessary. Much like the patch below:

diff --git a/src/wp-admin/includes/lazy-admin.php b/src/wp-admin/includes/lazy-admin.php
index e69de29bb2..b85b9ca001 100644
--- a/src/wp-admin/includes/lazy-admin.php
+++ b/src/wp-admin/includes/lazy-admin.php
+<?php
+
+// Ideally this file would be auto-generated
+
+function lazy_bootstrap_admin() {
+       require_once __DIR__ . '/admin.php';
+}
+
+// Bring the documentation over from the actual function
+function is_plugin_active($name) {
+       lazy_bootstrap_admin();
+       return \Admin\is_plugin_active($name);
+}
diff --git a/src/wp-admin/includes/plugin.php b/src/wp-admin/includes/plugin.php
index 6e332c247e..18aa080ef3 100644
--- a/src/wp-admin/includes/plugin.php
+++ b/src/wp-admin/includes/plugin.php
@@ -5,7 +5,7 @@
  * @package WordPress
  * @subpackage Administration
  */
-
+namespace Admin;
 /**
  * Parses the plugin contents to retrieve plugin's metadata.
  *

This would enable auto-loading of all the things, whether we're in the widgets editor world or any other area of WordPress. I am curious if that could help speed up the entire WP by avoiding eager loading in principle. The price to pay would be an additional layer of complexity, but this isn't far off from what we already do with JS and CSS build pipelines – only here we're in the PHP world.

Alternatively, we could deprecate the assumption that wp-admin has already been bootstrapped by the time widgets (or their forms) are being rendered. This doesn't solve anything for now, but it's a start.

I'm curious what y'all think. There's no way we can make it happen for 5.8 RC4 so for now let's just bootstrap the entire admin for each such request. We could potentially make it happen for 5.8.1 or 5.9 – if y'all think this is reasonable that is CC @peterwilsoncc @TimothyBJacobs @getdave @kevin940726 @noisysocks @youknowriad @hellofromtonya .

[hellofromtonya]

The fix here would be to bootstrap the whole admin during this request, which I don't really love. A fatal is pretty bad though.

I agree with @TimothyBJacobs that bootstrapping the whole admin is not ideal. There's a lot of functionality being loaded through the admin bootstrapper which may not be needed for this request. For example, the specific reported problem needs the function is_plugin_active which is in the wp-admin/includes/plugin.php file.

That said, functions not loaded into memory when using the widgets editor is a backwards compat issue. It can result in 3rd party fatal error. Using classic widgets, the admin is bootstrapped when loading the widgets screen and with admin-ajax.

[hellofromtonya]

The only other way I can think of is having an auto-generated file with "proxy functions" that would only load the admin when that's necessary. Much like the patch below:

Autoloading functions when needed has been a hot topic in the PHP world. There's a RFC dating back to 2015. Autoloading functions themselves requires some structural changes and more code to be added such as:

• Moving each function's code to a class with static public methods and then autoloading the class
• Creating a proxy or function mapper as Adam showed
• and some hacky ways such as through magic methods to automagically identify which file to load

But regardless, the original function name needs to be in memory before being invoked. Any other solution adds code to be maintained and debugged, hard drive space, memory, and processing time.

WordPress does not use PHP autoloaders. Rather, it uses the strategy of loading what's needed either through early bootstrapping or before the functionality is used in its context.

That's not say autoloaders can't be explored. Rather, my point is: it's outside the scope of this issue and likely needs discussion back in Core.

[adamziel]

Moving each function's code to a class with static public methods and then autoloading the class

I considered that, but then we'd still need proxies/mappers to preserve the existing function-based API :(

That's not say autoloaders can't be explored. Rather, my point is: it's outside the scope of this issue and likely needs discussion back in Core.

That's a good point, let's move over to core Trac: https://core.trac.wordpress.org/ticket/53670#ticket

[TimothyBJacobs]

Can someone clarify if this only breaks the widget that is using an admin function? Or will it pollute the entire editor?

[hellofromtonya]

After discussion in slack #core-editor channel, it was agreed to revert PR #33454. Instead, agreed to:

• Provide guidance to widget authors via dev note: when using a wp-admin function, load the appropriate core file within the plugin
• Monitor any reports of similar issues to determine if additional steps are needed

[adamziel]

Just surfacing my comment from Trac 53670 – it seems like the effect of #33454 on speed and memory consumption is minuscule.

[noisysocks]

Can we close this issue? What's remaining? 🙂

[peterwilsoncc]

Can we close this issue? What's remaining? 🙂

Yes, it can be closed. Based on various discussion it was decided not to fix it.

It may be worth a post on make.wordpress that legacy widget forms no longer support admin functions. Either on core or on the plugins blog.

[noisysocks]

👍 I added a small note to https://make.wordpress.org/core/2021/06/29/block-based-widgets-editor-in-wordpress-5-8/.

[JJJ]

It may be worth a post on make.wordpress that legacy widget forms no longer support admin functions. Either on core or on the plugins blog.

The opposite of this is also true – the new widget screen expects for all non-admin widget related functions to be available from within the admin context.

In my experience, this means that plugins that use is_admin() to avoid including theme-side PHP files will not have their widget output functions loaded for the live preview that now occurs, resulting in the same type of fatal error that @peterwilsoncc reported here.

This ultimately is because widgets.php doesn't iframe the theme for its preview the way that the Customizer does, leaving WP_ADMIN defined as true and WP_USE_THEMES undefined as null.

I'm unsure how common of a practice it is to split loading like this, but I maintain at least 1 plugin so far that does, which lead me here. 💜