Integrate compatibility related functions for the new editor

[desrosj]

In gutenberg/lib/compat.php, there are several functions that ensure compatibility with the new editor. These should be merged into WordPress Core.

[pento]

In 43758:

Classic Editor: Disable the wpautop TinyMCE plugin on block posts.

As the block editor adds its own <p> tags, disabling the wpautop stops the classic editor from removing them.

See #45113.

[pento]

Done/not needed:

• _gutenberg_utf8_split() doesn't need to be ported, as it's only used by the spec parser, not the default parser.
• gutenberg_disable_editor_settings_wpautop() was ported in [43758].
• gutenberg_wpautop() was ported in [43752].
• gutenberg_warn_classic_about_cloudflare() won't be ported.

I haven't ported gutenberg_add_rest_nonce_to_heartbeat_response_headers() yet, the current naming doesn't fit in with the structure of the other nonces in wp_refresh_post_nonces(), but we'll need to change the name in @wordpress/api-fetch to match.

I haven't ported the warning that shows up when trying to open a block post in the classic editor, I'm leaning towards putting that in the Classic Editor plugin. @karmatosed, do you have thoughts on this?

[karmatosed]

I haven't ported the warning that shows up when trying to open a block post in the classic editor, I'm leaning towards putting that in the Classic Editor plugin. @karmatosed, do you have thoughts on this?

If I understand correctly you are suggesting putting the warning in the classic editor? If that's the case then it makes sense to me assuming the plugin would have to be active to get that warning?

[pento]

Yah, you'd need to have the plugin active to get the warning. Not having the plugin would mean that no warning is shown when a block post is opened in the classic editor, which will likely cause unexpected behaviour.

[pento]

@adamsilverstein: Do you want the rest-api nonce to be put into core as is? I saw you were reviewing ​a bug in the nonce refreshing. I was originally unsure about it being in the top level of $response, but I'm not really fussed.

[adamsilverstein]

@pento yes, i think it makes sense to have this in core. I'll try to get back to the gutenberg ticket soon.

[pento]

@dd32: Since you wonderfully managed ​GB11347, would you mind having a look at getting the nonce refreshes included in core?

[dd32]

Replying to pento:

I haven't ported gutenberg_add_rest_nonce_to_heartbeat_response_headers() yet, the current naming doesn't fit in with the structure of the other nonces in wp_refresh_post_nonces(), but we'll need to change the name in @wordpress/api-fetch to match.

While working on this, I realised that wp_refresh_post_nonces() is no longer being used, as that code path is classic-editor only. As a result, the heartbeat nonce hasn't been being refreshed either (As that's been handled by wp-admin/includes/post.js in the past).

45113.heartbeatnonce.diff​ takes a slightly different approach than what was previously done in core:

• The Heartbeat nonce updating is now handled by heartbeat.js itself, rather than relying upon the loaded editor
• heartbeat.js also updates the wp-api package nonce global if loaded
• heartbeat.js now includes a reliance upon wp-hooks to do both jQuery.trigger() and wp.hooks.doAction() on events, so api-fetch can hook in directly to retrieve the nonce.

I've included a line that's intended to be temporary, returning rest-nonce in addition to rest_nonce for compatibility with the current api-fetch while acknowledging we probably should rename that to be rest_nonce.

Although untested at time of this comment, these changes should work transparently with the current Gutenberg stable/master.

---

After I wrote and tested the above, I realised that the nonces mentioned in wp_refresh_post_nonces() are included within the HTML for the Block editor. However, I don't think they're actually used, and are just a vestige so maybe not needed?

• getpermalinknonce - Not included
• samplepermalinknonce - Included in HTML, seems unused.
• closedpostboxesnonce - Included in HTML, seems unused.
• _ajax_linking_nonce - Included in HTML, seems like the internal linking dialogue uses wp-json instead of utilising this with admin-ajax.php though.
• _wpnonce - Included in HTML (actually multiple fields named that, with different source nonces) and seems like it's not used either as all actions go through the rest api.

With the above in mind, the approach taken in 45113.heartbeatnonce.diff​ might be OK, or might need stepping back and having the existing JS for updating nonces extracted from post.js and moved into heartbeat.js or it's own file.

[adamsilverstein]

The changes in 45113.heartbeatnonce.diff​ look good, I can give that some testing later in my local with nonce expiration set very low to verify refresh is actually working.

I agree we can likely remove the unused nonces included in the html as long as we are sure they aren't used. I did a little research to see how/if we use these nonces:

I think we need these nonces (and keep them refreshed) for actions taken within the classic editor block:

_ajax_linking_nonce is for ajax requests made when using the link selection modal which is still available.

_wpnonce is used in wp.media, think we need to retain that, I'll try to confirm by looking for the nonce in requests

These I think can be dropped for GB:
samplepermalinknonce is used in the classic editor for slug operations, we probably don't need it for Gutenberg.

closedpostboxesnonce is used when persisting metabox open/closed states - we probably don't need for GB

I couldn't find any use of getpermalinknonce

[adamsilverstein]

@dd32 is this against the 5.0 branch or trunk? having trouble applying cleanly.

[dd32]

patch refresh

[dd32]

@adamsilverstein I've uploaded a refreshed patch, a few conflicts came in with some recent commits.
The patch is generated from https://core.svn.wordpress.org/branches/5.0 but should apply cleanly after the refresh.

[dd32]

Replying to adamsilverstein:

The changes in 45113.heartbeatnonce.diff​ look good, I can give that some testing later in my local with nonce expiration set very low to verify refresh is actually working.

I agree we can likely remove the unused nonces included in the html as long as we are sure they aren't used. I did a little research to see how/if we use these nonces:

I think we need these nonces (and keep them refreshed) for actions taken within the classic editor block:

_ajax_linking_nonce is for ajax requests made when using the link selection modal which is still available.

Yep, confirmed that's still in use. When links are inserted with the classic editor block they go via admin-ajax.php, rather than via wp-json in the rest of the blocks.

_wpnonce is used in wp.media, think we need to retain that, I'll try to confirm by looking for the nonce in requests

The _wpnonce in question was a edit_post_{$id} nonce, so I don't think it was being refreshed for the purposes of media, but I'll take another look into that.

Based on the above, I'm going to go back to the drawing board a little, and restore the existing nonce refreshing, but I'm not sure if i'll get to that today.

[adamsilverstein]

Confirming that in my testing using 5.0 with 45113.heartbeatnonce.2.diff​ nonces are refreshed correctly. I set nonce expiration to 25 seconds and heartbeat to 15 seconds and observed that saves continued to work and used each new nonce returned via the heartbeat.

re _wpnonce I only saw that when scanning the codebase, i'll do a little more digging to see if I can determine where/how it is used and if we still need it.

[adamsilverstein]

Note: to test that nonce refreshes work i’m setting heartbeat frequency ​https://gist.github.com/adamsilverstein/4bfc76454656046b24f18b292da895f7 and nonce expiration ​https://gist.github.com/adamsilverstein/cf39db696d242e6c1f2f7695442d7fc0 to testing intervals…

eg:

i set nonce expiration to 25 and heartbeat to 15. with every other heartbeat i can check the wp-ajax response in console and verify a new wp-nonce (and wp_nonce :() are returned
then in GB i click save draft or update and observe the reset request, verifying that the nonce included in the header is the same one that was returned from the heartbeat

then, i tried with the nonce expiration set to 25 and heartbeat set to 60
after 30 seconds, i hit save and it failed (as expected) because the nonce was not valid any more and had not been refreshed
the api responds rest_cookie_invalid_nonce

to complete testing, i'm investigating how nonces are used in wp.media to ensure those are being refreshed as well.

[adamsilverstein]

@dd32 Looking more closely, I think keeping _wpnonce refreshed would be useful because use it for post locking in Gutenberg ​https://github.com/WordPress/gutenberg/blob/56b9553e67f99e2dd51cbb680a9c664397717c72/packages/editor/src/components/post-locked-modal/index.js#L109] (and autosaves in classic - ​https://github.com/WordPress/wordpress-develop/blob/6b366c6620fdd5960cedcdf80955966a715efa82/src/js/_enqueues/wp/autosave.js#L730).

media uses nonces localized as nonces ​https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/media.php#L3288-L3296 - checking to see if these are used in GB.

[adamsilverstein]

testing the media modal in GB, i see nonces used in media for the following actions.

Once my nonces expired these actions all failed. Worth testing if these failures also occur in the classic editor, i'm not sure if they were ever being refreshed.

• updating captions ('save-attachment' action, pulls from nonces)
• uploading an image, i see a nonce used for the upload-attachment action, thats in _wpPluploadSettings and added here: ​https://github.com/WordPress/wordpress-develop/blob/b2374bf1ad30fa572d40cbe466388563f6fe2b70/src/wp-admin/includes/media.php#L1995
• clicking the delete image link in the modal fires the delete-post action, also pulls from 'nonces'

Although we should ensure all nonces are refreshed I don't think this is a blocker for merging - only a very small percentage of users will leave their browser window open on the editor for long enough for normal nonces to expire, and these extra nonces already are not refreshed, so the classic editor similarly will fail to perform media actions, eg this isn't a regression.

Next: testing again with refreshing in place to see if any of these are currently being refreshed.

[adamsilverstein]

verified nonces for wp.media are not refreshed in gb or in the classic editor. once my nonce expires, the media actions noted above all fail. Since this isn't a regression, I think we can address this in a separate ticket.

[matveb]

Thanks for following up on this one. Is this ready to be committed?

[adamsilverstein]

Yes - think it is, I was going to await @dd32 to see if he wanted to add/revise the latest patch. As is though, I verified nonce refreshing works and if merging this unblocks the RC i'd say go for it!

[youknowriad]

In 43939:

Block Editor: Refresh nonces used by wp.apiFetch.

Adds heartbeat nonces refreshing support to wp.apiFetch requests.

Props pento, adamsilverstein, dd32, desrosj.
Fixes #45113.

[desrosj]

In 44120:

Classic Editor: Disable the wpautop TinyMCE plugin on block posts.

As the block editor adds its own <p> tags, disabling the wpautop stops the classic editor from removing them.

Props pento.

Merges [43758] to trunk.

See #45113.

[desrosj]

In 44275:

Block Editor: Refresh nonces used by wp.apiFetch.

Adds heartbeat nonces refreshing support to wp.apiFetch requests.

Props pento, adamsilverstein, dd32, desrosj, youknowriad.

Merges [43939] into trunk.

Fixes #45113.

[aduth]

In 44949:

Scripts: Assign api-fetch nonce with corrected rest_nonce.

As of @wordpress/api-fetch@3.0.0 (introduced in 44812), the apiFetch nonce middleware must have its nonce value assigned explicitly, and will no longer listen for heartbeat ticks automatically. This changeset adds an inline script for the default registration of the api-fetch script handle to assign the nonce value in response to the heartbeat action. In doing so, it removes the now-unused, misnamed rest-nonce property from the heartbeat response, whose original introduction served as temporary compatibility with earlier versions of @wordpress/api-fetch.

See ​https://github.com/WordPress/gutenberg/pull/13451
See #45113

Props adamsilverstein, nerrad .
Fixes #46107 .