Block Theme Previews: Add a nonce for activation

[scruffian]

This makes a few changes to block theme previews:

1. Adds a nonce so that activation works. This is needed until we have a REST API endpoint for activating themes
2. Loads the theme-previews code in both wp-admin and in the REST API. This means moving the include to wp-settings.php - there may be a better place for this, the point is that we need to apply the filters in both wp-admin and in the API, so we need to move it out of admin-filters. I also moved the file itself out of wp-admin into wp-includes, since we use it in more situations.

Also ensure that we always load the filters even outside of wp-admin.

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

---

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.

[jomonthomaslobo]

Thanks for the fix

[peterwilsoncc]

This looks good to me with a couple of suggestions inline.

I've suggested prefixing the constant name with WP_ (putting it in the wp global could also work), would such a change require an upstream change in the Gutenberg repo?

[bph]

@scruffian / @peterwilsoncc would this make it into 6.3 still?
here is the trac ticket:
https://core.trac.wordpress.org/ticket/58712
cc @tellthemachine @azaozz ?

[scruffian]

This needs to go into 6.3, or we need to take block theme previews out of 6.3.

[peterwilsoncc]

@scruffian @bph I submitted the PR WordPress/gutenberg#52398 to rename the variable to use an WP_ prefix. This is good to go in to 6.3 but the upstream PR will need to be included at the same time.

[peterwilsoncc]

LGTM, thanks folks.

[tellthemachines]

Committed in r56199 / 7813524

[anton-vlasenko]

It seems I'm a bit late :) Nevertheless, I have added my code review below. Thanks!

[anton-vlasenko]

Nitpick: I don't see many PHP frameworks using inline tags to output text these days. Instead, I would use echo or print to output the nonce code to the buffer. But I admit, this is just a personal preference.

[anton-vlasenko]

This could potentially be a security issue. Is there a specific reason for not escaping the nonce value?

Suggested change

	window.WP_BLOCK_THEME_ACTIVATE_NONCE = '<?php echo wp_create_nonce( $nonce_handle ); ?>';
	window.WP_BLOCK_THEME_ACTIVATE_NONCE = '<?php echo esc_js( wp_create_nonce( $nonce_handle ) ); ?>';

[peterwilsoncc]

Good catch @anton-vlasenko -- it's more a bug than a security issue as it's not user input, however it may break with custom nonce implementations using special characters.

It will need to change to = <?php echo wp_json_encode( wp_create_nonce( $nonce_handle ) ); (without quotes) as esc_js() is intended for use in DOM attributes.

wp> esc_js( 'Pens & Pencils' );
=>  string(18) "Pens & Pencils"
wp> wp_json_encode( 'Pens & Pencils' );
=> string(16) ""Pens & Pencils""

I'll reopen the ticket and add a PR to the GB repo.

[anton-vlasenko]

Thank you for getting back to my code review, @peterwilsoncc!

Yes, using wp_json_encode() is also an option.
In fact, I prefer wp_json_encode() over esc_js() because it automatically adds quotes.
I was just uncertain about which function aligns better with the "true WordPress way" of doing things.