Skip to content

Check that get_current_screen is callable #25935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
noahtallen merged 1 commit into from
Oct 8, 2020
Merged

Check that get_current_screen is callable #25935

noahtallen merged 1 commit into from
Oct 8, 2020

Conversation

@noahtallen
Copy link
Member

@noahtallen noahtallen commented Oct 8, 2020 •
edited
Loading

Description

Checks that get_current_screen is callable before calling it.

This was causing PHP notice to show up on the site front end, and prevented any content from displaying:

  1. wp-env clean all
  2. wp-env start --update
  3. (obviously logged out because of the above.

This also appears to resolve the e2e issue described here: #25826 (comment)

How has this been tested?

With this change, I do not see an error in the above scenario.

Screenshots

the error is:

Fatal error: Uncaught Error: Call to undefined function get_current_screen() in /var/www/html/wp-content/plugins/gutenberg/lib/widgets-page.php:136 Stack trace: #0 /var/www/html/wp-includes/class-wp-hook.php(287): gutenberg_widgets_editor_load_block_editor_scripts_and_styles(false) #1 /var/www/html/wp-includes/plugin.php(206): WP_Hook->apply_filters(false, Array) #2 /var/www/html/wp-includes/script-loader.php(2232): apply_filters(‘should_load_blo...‘, false) #3 /var/www/html/wp-includes/script-loader.php(2246): wp_should_load_block_editor_scripts_and_styles() #4 /var/www/html/wp-includes/class-wp-hook.php(287): wp_enqueue_registered_block_scripts_and_styles(‘’) #5 /var/www/html/wp-includes/class-wp-hook.php(311): WP_Hook->apply_filters(‘’, Array) #6 /var/www/html/wp-includes/plugin.php(478): WP_Hook->do_action(Array) #7 /var/www/html/wp-includes/script-loader.php(2208): do_action(‘enqueue_block_a...‘) #8 /var/www/html/wp-includes/class-wp-hook.php(287): wp_common_block_scripts_and_styles(‘’) #9 /var/www/html/wp-includes/c in /var/www/html/wp-content/plugins/gutenberg/lib/widgets-page.php on line 136

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

Addison-Stavlo
Addison-Stavlo approved these changes Oct 8, 2020
View reviewed changes
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the failing tests for me locally.

@noahtallen noahtallen mentioned this pull request Oct 8, 2020
Merged
6 tasks
@github-actions
Copy link

github-actions bot commented Oct 8, 2020

Size Change: 0 B

Total Size: 1.18 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.52 kB 0 B
build/api-fetch/index.js 3.35 kB 0 B
build/autop/index.js 2.72 kB 0 B
build/blob/index.js 668 B 0 B
build/block-directory/index.js 8.55 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 129 kB 0 B
build/block-editor/style-rtl.css 10.9 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 8.65 kB 0 B
build/block-library/editor.css 8.65 kB 0 B
build/block-library/index.js 135 kB 0 B
build/block-library/style-rtl.css 7.66 kB 0 B
build/block-library/style.css 7.65 kB 0 B
build/block-library/theme-rtl.css 741 B 0 B
build/block-library/theme.css 741 B 0 B
build/block-serialization-default-parser/index.js 1.78 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 47.5 kB 0 B
build/components/index.js 170 kB 0 B
build/components/style-rtl.css 15.4 kB 0 B
build/components/style.css 15.4 kB 0 B
build/compose/index.js 9.42 kB 0 B
build/core-data/index.js 12 kB 0 B
build/data-controls/index.js 685 B 0 B
build/data/index.js 8.6 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.42 kB 0 B
build/edit-navigation/index.js 10.7 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.29 kB 0 B
build/edit-post/style.css 6.27 kB 0 B
build/edit-site/index.js 20.8 kB 0 B
build/edit-site/style-rtl.css 3.71 kB 0 B
build/edit-site/style.css 3.72 kB 0 B
build/edit-widgets/index.js 21.4 kB 0 B
build/edit-widgets/style-rtl.css 3 kB 0 B
build/edit-widgets/style.css 3 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.5 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.84 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 734 B 0 B
build/format-library/index.js 7.48 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 1.74 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.54 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.39 kB 0 B
build/keycodes/index.js 1.85 kB 0 B
build/list-reusable-blocks/index.js 3.02 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.12 kB 0 B
build/notices/index.js 1.69 kB 0 B
build/nux/index.js 3.27 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.44 kB 0 B
build/primitives/index.js 1.34 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13 kB 0 B
build/server-side-render/index.js 2.6 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.24 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.74 kB 0 B
build/warning/index.js 1.13 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@noahtallen noahtallen merged commit 7bf5063 into master Oct 8, 2020
@noahtallen noahtallen deleted the add/check-for-function-callable branch October 8, 2020 00:52
@github-actions github-actions bot added this to the Gutenberg 9.2 milestone Oct 8, 2020
@TimothyBJacobs
Copy link
Member

TimothyBJacobs commented Oct 8, 2020

Do we know why this function is getting called so early before the screen is loaded? It seems like it might be papering over another bug if that function would've returned true had it been queried later in the request.

@noisysocks
Copy link
Member

noisysocks commented Oct 8, 2020

Yes, why is get_current_screen() not callable? This doesn't look like we're fixing the root cause.

@noahtallen
Copy link
Member Author

noahtallen commented Oct 8, 2020

🤷 the other option was reverting #25826. It could have to do with when that filter is being called?

@noahtallen
Copy link
Member Author

noahtallen commented Oct 8, 2020

I.e. it seems that the filter should_load_block_editor_scripts_and_styles is being called on front-end screens. is get_current_screen available to logged-out users outside of wp-admin?

@noahtallen
Copy link
Member Author

noahtallen commented Oct 8, 2020

My current guess is that the root cause would be in WordPress core, given that should_load_block_editor_scripts_and_styles is not called from the plugin

@noahtallen
Copy link
Member Author

noahtallen commented Oct 8, 2020

There is also precedence for this here:

gutenberg/lib/widgets.php

Lines 14 to 19 in 41c8464

// If get_current_screen does not exist, we are neither in the standard block editor for posts, or the widget block editor.
// We can safely return false.
if ( ! function_exists( 'get_current_screen' ) ) {
return false;
}
$screen = get_current_screen();

@noisysocks
Copy link
Member

noisysocks commented Oct 8, 2020

Right, right. I think should_load_block_editor_scripts_and_styles should not be called on non-admins screens. I'll open a patch.

@noisysocks
Copy link
Member

noisysocks commented Oct 8, 2020

Fixed in https://core.trac.wordpress.org/ticket/51330. This PR should now be unnecessary if running trunk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Addison-Stavlo Addison-Stavlo Addison-Stavlo approved these changes

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey

@TimothyBJacobs TimothyBJacobs Awaiting requested review from TimothyBJacobs

@adamziel adamziel Awaiting requested review from adamziel

@noisysocks noisysocks Awaiting requested review from noisysocks

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Gutenberg 9.2

Development

Successfully merging this pull request may close these issues.

5 participants

@noahtallen @TimothyBJacobs @noisysocks @Addison-Stavlo