Skip to content

Prevent PHP notice in Customizer when using block theme#3230

Closed
westonruter wants to merge 2 commits intoWordPress:trunkfrom
westonruter:trac-54905
Closed

Prevent PHP notice in Customizer when using block theme#3230
westonruter wants to merge 2 commits intoWordPress:trunkfrom
westonruter:trac-54905

Conversation

@westonruter
Copy link
Copy Markdown
Member

The issue was introduced in #2217 where a WP_Customize_Nav_Menus_Panel::check_capabilities() method override was introduced which caused the return to be false if the current theme doesn't support menus nor widgets.

The problem is that since check_capabilities returns false, when WP_Customize_Manager::prepare_controls() runs it will remove any registered panels that the user doesn't have the capability to manage.

Ironically, I actually reviewed that PR and I thought the approach would work, but clearly it doesn't. I did provide an alternative solution, however, which should actually fix the issue here: #2217 (comment)

Trac ticket: https://core.trac.wordpress.org/ticket/54905


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@noisysocks
Copy link
Copy Markdown
Member

Makes sense. Marking the panel as inactive instead of removing it would indeed make it so that calls to get_panel( 'nav_menus' ) don't return an empty value.

What I'm wondering though is: why do we even run WP_Customize_Nav_Menus::available_items_template, WP_Customize_Widgets::output_widget_control_templates, and WP_Customize_Nav_Menus::enqueue_scripts when the theme doesn't support menus nor widgets? It doesn't seem to me that there should ever be a call to get_panel( 'nav_menus' ) when we know that there won't be a panel with that ID.

@westonruter
Copy link
Copy Markdown
Member Author

Yes. Really the Widgets and Nav Menus components should be disabled entirely. There is a filter for that: customize_loaded_components. However, I do not believe it will work here because it has to run when WP_Customize_Manager is constructed which happens at plugins_loaded, and thus before the theme is loaded. This means it would run too early to check if wp_is_block_theme(), right?

@noisysocks
Copy link
Copy Markdown
Member

Yeah that's right. Bummer!

OK, marking as inactive seems reasonable all things considered 👍

* trunk: (298 commits)
  Media: improve image engine detection when using the output format filter.
  Blocks: Avoid extra calls to `realpath()` in block scripts and styles registration.
  Administration: Guard against undefined `$GLOBALS['hook_suffix']` in `WP_Screen::get()`.
  Twenty Nineteen: Ensure Pullquote Block text color is reflected on front-end.
  Docs: Various docblock fixes in `global-styles-and-settings.php`, as per documentation standards.
  Docs: Various docblock fixes in `WP_Theme_JSON` class, as per documentation standards.
  Query: Move call to `update_menu_item_cache` in `WP_Query`
  Fix: cite styles declared via theme.json not working.
  Editor: Add missing `blocks` origin to `theme.json`.
  Build/Test tools: Add tests for `wp_nonce_url()`.
  Build/Test Tools: Use `require_once` instead of `require`.
  Bundled Themes: Properly escape URLs.
  Twenty Eleven: Pass template directory URLs through `esc_url()`.
  Tests: Clean up test image for site icon in `Tests_REST_Server` on teardown.
  Tests: Replace some occurrences of `assertEquals()` with `assertSame()`.
  Tests: Bring some consistency to `WP_Image_Editor_GD` and `WP_Image_Editor_Imagick` tests.
  Build/Test Tools: Update npm dependencies to their latest versions.
  Editor: Ensure block styles in `theme.json` are rendered.
  Themes: Replace `array_map()` usage in `WP_Theme_JSON::get_default_slugs()`.
  Users: Revert use of shared objects for current user.
  ...
@westonruter
Copy link
Copy Markdown
Member Author

Committed to trunk in https://core.trac.wordpress.org/changeset/54419

@westonruter westonruter closed this Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants