Skip to content
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.

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

Allow remote pattern registration in theme.json when core patterns are disabled #1

Merged

Conversation

ndiego
Copy link
Owner

@ndiego ndiego commented Jun 30, 2022

In 6.0 we now have functionality that allows you to register selected remote patterns (from the Pattern Directory) via theme.json. While this is great, it requires you to fully enable all core and remote patterns. This can be a blocker to adoption.

Many theme authors want to have all core and remote patterns disabled by default using remove_theme_support( 'core-block-patterns' ). This ensures they are serving only patterns relevant to their theme to customers/clients.

This PR/Trac ticket removes the theme support and remote filter checks from the function that registers remote patterns specified in theme.json. This allows theme authors to disable core patterns but then selectively register the ones they want from the Pattern Directory. We assume that if a theme author intentionally specifies patterns in theme.json, they intend for them to be present in the pattern inserter and the checks are not required.

This change will make the Pattern Directory more appealing/usable from the theme author's perspective.

Note, that while this issue is technically not a "bug", it is a pretty large blocker for Pattern Directory adoption and I feel should be considered for 6.0.1.

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


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.

@ndiego ndiego added the enhancement New feature or request label Jun 30, 2022
@ndiego ndiego merged commit e7de8e3 into trunk Jun 30, 2022
@ndiego ndiego deleted the support-themejson-patterns-when-core-patterns-disabled branch June 30, 2022 13:36
ndiego pushed a commit that referenced this pull request Dec 17, 2022
…ize_Manager::get_return_url()`.

This particular code block only makes sense to run when `$this->return_url` is not null. Previously, it caused a "passing null to non-nullable" deprecation notice on PHP 8.1.

By moving the code into the `if ( $this->return_url )` condition block, the code will only be run when `$this->return_url` contains a non-falsey/non-null value.

No additional tests added as this issue was found via the existing tests for the function containing the bug.

This solves the following two PHP 8.1 test errors:
{{{
1) Tests_WP_Customize_Manager::test_return_url
parse_url(): Passing null to parameter #1 ($url) of type string is deprecated

/var/www/src/wp-includes/class-wp-customize-manager.php:4696
/var/www/tests/phpunit/tests/customize/manager.php:2975
/var/www/vendor/bin/phpunit:123

2) Tests_WP_Customize_Manager::test_customize_pane_settings
parse_url(): Passing null to parameter #1 ($url) of type string is deprecated

/var/www/src/wp-includes/class-wp-customize-manager.php:4696
/var/www/src/wp-includes/class-wp-customize-manager.php:4898
/var/www/tests/phpunit/tests/customize/manager.php:3085
/var/www/vendor/bin/phpunit:123
}}}

Follow-up to [46754].

Props jrf, costdev.
See #55656.

git-svn-id: https://develop.svn.wordpress.org/trunk@54135 602fd350-edb4-49c9-b593-d223f7449a82
ndiego pushed a commit that referenced this pull request Dec 17, 2022
…pc_server::mw_newPost()`.

The `wp_xmlrpc_server::mw_newPost()` method creates a new post via `wp_insert_post()`, but the default/fallback values used in the function were not in line with the default/fallback values used in the `wp_insert_post()` function.

The `wp_insert_post()` function does a `wp_parse_args()` (array merge) of the received arguments with the defaults. If any of the received arguments are `null`, this would overwrite the default value, as seen in [https://3v4l.org/bfVlv array_merge() example], and lead to "passing null to non-nullable" deprecation notices on PHP 8.1 for certain arguments.

This commit:
* Ensures that all arguments are defined before they are `compact()`'ed together to the arguments array.
* Verifies that the default/fallback value of the arguments as set within the `wp_xmlrpc_server::mw_newPost()` method are the same as the default/fallback values used in the `wp_insert_post()` function.
* Verifies that arguments which do not have a default/fallback value defined in the `wp_insert_post()` function are handled correctly.
 * This was not the case for `$post_name`, which would previously already get an empty string default value in the `wp_xmlrpc_server::mw_newPost()` function, but then in the `wp_insert_post()` function, this would prevent the slug generation from being activated. Fixed now by setting the default in the `wp_xmlrpc_server::mw_newPost()` function to `null`.
 * The `page_template` argument was handled, but not documented in the `wp_insert_post()` function. The argument is now documented in the `wp_insert_post()` function DocBlock. Note: There are more than likely several other potential arguments missing from that list, but verifying the whole list is outside the scope of this particular commit.

Includes minor simplifications, such as:
* Setting a default ahead of an `if`, instead of in an `else` clause (as long as no function call is needed to set the default).
* Removing the unnecessary logic duplication in the `$post_status` switch.
* Using a combined concatenation + assignment operator for adding `$post_more`.

Fixes various errors along the lines of:
{{{
1) Tests_XMLRPC_mw_editPost::test_draft_not_prematurely_published
strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated

/var/www/src/wp-includes/formatting.php:2497
/var/www/src/wp-includes/class-wp-hook.php:308
/var/www/src/wp-includes/plugin.php:205
/var/www/src/wp-includes/post.php:2835
/var/www/src/wp-includes/post.php:2720
/var/www/src/wp-includes/post.php:4066
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:5616
/var/www/tests/phpunit/tests/xmlrpc/mw/editPost.php:315

...

23) Tests_XMLRPC_mw_editPost::test_draft_not_prematurely_published
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/var/www/src/wp-includes/kses.php:2074
/var/www/src/wp-includes/class-wp-hook.php:307
/var/www/src/wp-includes/plugin.php:205
/var/www/src/wp-includes/post.php:2835
/var/www/src/wp-includes/post.php:2720
/var/www/src/wp-includes/post.php:4066
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:5615
/var/www/tests/phpunit/tests/xmlrpc/mw/editPost.php:315
/var/www/vendor/bin/phpunit:123
}}}

Follow-up to [1563], [4793], [7900], [16824], [19848], [40677], [51968].

Props jrf.
See #55656.

git-svn-id: https://develop.svn.wordpress.org/trunk@54320 602fd350-edb4-49c9-b593-d223f7449a82
ndiego pushed a commit that referenced this pull request Dec 17, 2022
…pc_server::_insert_post()`.

The `wp_xmlrpc_server::_insert_post()` method creates a new post via `wp_insert_post()` or updates an existing one via `wp_update_post()`, which subsequently calls `wp_insert_post()`. However, the default/fallback values used in the function were not in line with the default/fallback values used in the `wp_insert_post()` function.

The `wp_insert_post()` function does a `wp_parse_args()` (array merge) of the received arguments with the defaults. If any of the received arguments are `null`, this would overwrite the default value, as seen in [https://3v4l.org/bfVlv array_merge() example], and lead to "passing null to non-nullable" deprecation notices on PHP 8.1 for certain arguments.

Unfortunately, the conditional logic within the `wp_xmlrpc_server::_insert_post()` function itself often uses an `isset()` to trigger certain code blocks, so syncing the defaults with those used in the `wp_insert_post()` function was not an option.

This commit:
* Updates the default/fallback values in the `$defaults` array only for those values where this would not lead to a change in the behavior of the function.
* Adds a safeguard function, filtering out all remaining `null` values from the `$post_data` array before it is passed on to the `wp_insert_post()` or `wp_update_post()` functions. Removing those values is safe as this means that these array keys will now:
 * either be set to the default/fallback value as defined in `wp_insert_post()`.
 * or not be set and for those values which don't have a default/fallback value in `wp_insert_post()`, the function does an `! empty()` or `isset()` check anyway and those array keys not being defined means that the result of those checks will remain the same.

Includes
* Removing a couple of conditions which are now redundant.
* Removing an `expectDeprecation()` in the `Tests_Date_XMLRPC` test class, which is now no longer needed.

Fixes various errors along the lines of:
{{{
36) Tests_XMLRPC_wp_newPost::test_no_content
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/var/www/src/wp-includes/kses.php:2074
/var/www/src/wp-includes/class-wp-hook.php:307
/var/www/src/wp-includes/plugin.php:205
/var/www/src/wp-includes/post.php:2835
/var/www/src/wp-includes/post.php:2720
/var/www/src/wp-includes/post.php:4066
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:1683
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:1347
/var/www/tests/phpunit/tests/xmlrpc/wp/newPost.php:25
/var/www/vendor/bin/phpunit:123
}}}

Follow-up to [1563], [4793], [7900], [16824], [19848], [19873], [20632], [40677], [51968], [54320].

Props jrf.
See #55656.

git-svn-id: https://develop.svn.wordpress.org/trunk@54321 602fd350-edb4-49c9-b593-d223f7449a82
ndiego pushed a commit that referenced this pull request Dec 17, 2022
…ions path.

The `$path` parameter of `load_script_textdomain()` had a default value of `null`, but would be passed onto the `untrailingslashit()` function without any input validation, even though the latter explicitly only expects/supports a string input.

This commit changes the default value for `$path` to an empty string, and adds an `is_string()` check before passing the value to `untrailingslashit()` to fix the issue at the point where the invalid input is incorrectly (not) validated.

Note: Changing the `untrailingslashit()` function is outside the scope of this commit.

Includes:
* Adding a dedicated unit test for this issue.
* Correcting the default value for `$path` from `null` to an empty string in a few related methods and functions:
 * `WP_Dependency::set_translations()`
 * `WP_Scripts::set_translations()`
 * `wp_set_script_translations()`
 * `load_script_textdomain()`

This fix also allows to remove a couple of calls to `expectDeprecation()` in unrelated tests.

Fixes an error when running the test suite:
{{{
4) Tests_Dependencies_Scripts::test_wp_external_wp_i18n_print_order
rtrim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/formatting.php:2782
/var/www/src/wp-includes/l10n.php:1068
/var/www/src/wp-includes/class.wp-scripts.php:605
/var/www/src/wp-includes/class.wp-scripts.php:320
/var/www/src/wp-includes/class.wp-dependencies.php:136
/var/www/src/wp-includes/functions.wp-scripts.php:109
/var/www/tests/phpunit/tests/dependencies/scripts.php:1505
/var/www/tests/phpunit/includes/utils.php:436
/var/www/tests/phpunit/tests/dependencies/scripts.php:1507
/var/www/vendor/bin/phpunit:123
}}}

Follow-up to [44169], [44607], [51968].

Props jrf, ocean90, Chouby, swissspidy, lovor, iviweb, meysamnorouzi, DarkoG, oneearth27, SergeyBiryukov.
Fixes #55967. See #55656.

git-svn-id: https://develop.svn.wordpress.org/trunk@54349 602fd350-edb4-49c9-b593-d223f7449a82
ndiego pushed a commit that referenced this pull request Dec 17, 2022
…_JSON::get_property_value()`.

This commit aims to fix errors caused by incorrect usage of the `strncmp()` function inside the `WP_Theme_JSON::get_property_value()` method on PHP 8.1 and above.

Some history of the affected code:
* [50973] introduced the `WP_Theme_JSON::get_property_value()` method.
* [54162] removed the `$default` parameter from the `_wp_array_get()` call in the method.

With the latter change, the default value that is returned if the path does not exist within the array, or if `$array` or `$path` are not arrays, became `null` instead of an empty string. Since `null` would then be unintentionally passed to the `strncmp()` function further in the code, this caused ~35 errors in the test suite along the lines of:
{{{
1) Tests_Blocks_Editor::test_get_block_editor_settings_theme_json_settings
strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated

/var/www/src/wp-includes/class-wp-theme-json.php:1754
/var/www/src/wp-includes/class-wp-theme-json.php:1641
/var/www/src/wp-includes/class-wp-theme-json.php:2066
/var/www/src/wp-includes/class-wp-theme-json.php:1002
/var/www/src/wp-includes/class-wp-theme-json.php:898
/var/www/src/wp-includes/global-styles-and-settings.php:140
/var/www/src/wp-includes/block-editor.php:421
/var/www/tests/phpunit/tests/blocks/editor.php:388
/var/www/vendor/bin/phpunit:123
}}}

This commit includes:
* Restoring the `$default` value for `_wp_array_get()` call.
* Adding an early return if the value is an empty string or `null`.
* Adding a dedicated unit test to ensure that the method returns a string for invalid paths or `null` values.

Follow-up to [50973], [54162].

Props antonvlasenko, jrf, imadarshakshat, SergeyBiryukov.
Fixes #56620.

git-svn-id: https://develop.svn.wordpress.org/trunk@54362 602fd350-edb4-49c9-b593-d223f7449a82
ndiego pushed a commit that referenced this pull request Dec 17, 2022
…L10n`.

These three tests for `wp_dashboard_recent_drafts()` would run into a PHP 8.1 "passing null to non-nullable" deprecation for the call to `ltrim()` when the result of `get_edit_post_link()` is passed to `esc_url()`.

Setting a deprecation expectation would not solve this as the returned value would still not match the expected value(s).

The recent drafts list is only displayed on the Dashboard screen for users with the `edit_posts` capability. By setting the current user to Editor, the prerequisites for `wp_dashboard_recent_drafts()` are met, which means the deprecation notice is avoided and the assertions will succeed.

This commit addresses a few errors in the test suite along the lines of:
{{{
1) Tests_L10n::test_length_of_draft_should_be_counted_by_words
ltrim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/formatting.php:4376
/var/www/src/wp-admin/includes/dashboard.php:657
/var/www/tests/phpunit/tests/l10n.php:449
/var/www/vendor/bin/phpunit:123
}}}

Follow-up to [45505], [52253], [52259].

Props jrf, desrosj, SergeyBiryukov.
See #56681, #55652, #55656.

git-svn-id: https://develop.svn.wordpress.org/trunk@54365 602fd350-edb4-49c9-b593-d223f7449a82
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant