Add availability checks to widget related code #15983
Conversation
jorgefilipecosta
added
[Type] Bug
jorgefilipecosta
requested review from
aduth,
gziolo and
youknowriad
as code owners
| @@ -11,6 +11,11 @@ | |||
| * @return boolean True if a screen containing the block editor is being loaded. | |||
| */ | |||
| function gutenberg_is_block_editor() { | |||
There was a problem hiding this comment.
Aside: This feels like it's not specific to widgets. Might make more sense surfaced up to the top-level gutenberg.php file. Then again, it's only necessary vs. WP_Screen::is_block_editor since the latter doesn't account for the widgets screen. I guess this is something we'll need to sort out sooner than later.
There was a problem hiding this comment.
It was kept on the widgets.php file temporarily to make merging Gutenberg to core easier (this file is not merged). The final solution should be updating WP_Screen::is_block_editor when things get more stable.
lib/widgets.php
Outdated
| 'isCallbackWidget' => false, | ||
| 'isHidden' => in_array( $class, $core_widgets, true ), | ||
| ); | ||
| if ( ! empty( $GLOBALS['wp_widget_factory'] ) ) { |
There was a problem hiding this comment.
Minor: Would calling global $wp_widget_factory; when the global isn't assigned be problematic, and then proceed to check ! empty( $wp_widget_factory ) ? Is only a minor difference to avoid reference to the superglobal.
There was a problem hiding this comment.
In my tests that approach also seems to work well, the code was updated 👍
jorgefilipecosta
force-pushed
the
add/availability-checks-to-widgets-code
branch
from
1ab42c4 to
8919827
Compare
jorgefilipecosta
force-pushed
the
add/availability-checks-to-widgets-code
branch
from
8919827 to
5a8a715
Compare
Description
This PR adds availability checks for the function get_current_screen and for the globals wp_widget_factory, wp_registered_widgets.
The function get_current_screen should only be called in admin pages, but according to the docs, we may have admin pages without the function so I think a check should be added. For example, I guess it may be possible to have an admin page with admin_footer but without the function defined.
Regarding the globals wp_widget_factory, wp_registered_widgets used in gutenberg_get_legacy_widget_settings function, the function is called as part of the filter block_editor_settings, I guess the filter may be executed in a situation where the widget globals are not initialized e.g: in a custom plugin page with a custom block editor. I think it is better to check the availability of the globals. If the globals are not available we will not pass available legacy widgets to the front end.
How has this been tested?
I verified there are no noticeable changes to the way legacy widgets and block editor widget screen works.
I added
|| trueand&& falseto the function checks and global checks respectively. And I verified the noticeable effect was that no available legacy widgets exist, and if I add the following widget code<!-- wp:legacy-widget {"identifier":"WP_Widget_Media_Image","instance":{"title":"My image"},"isCallbackWidget":false} /-->to the post editor nothing is rendered, the widget just renders hidden fields on the server and then the client renders the UI with JS but by not validating the condition gutenberg_is_block_editor we stopped loading that scripts. If I remove the condition the legacy widget should load correctly on the post editor.