Blank Page When $_GET['postId'] Does Not Exist in /wp-admin/site-editor.php

[benniledl]

Hello,

In WordPress 6.6, there has been a change in behavior when the passed postId does not exist in /wp-admin/site-editor.php. Previously, the editor sidebar would appear as usual, while the page preview on the right would be blank. Users could simply click back in the editor sidebar and navigate to the desired content.

However, starting with version 6.6, the entire page becomes blank, which can be confusing for users. This issue might occur due to several reasons:

The content intended for editing has just been deleted.
The postId for the blog homepage, such as twentytwentyfourhome, gets modified for example by the waf due to concerns like path traversal, removing the double slash.

The editor should appear as usual even when the postId is invalid and just display an error message in the content preview.

[mi5t4n]

Hello,

WP 6.6.1
Theme: Twenty Twenty-Three

If we navigate to non-existtent postID on the site editor /wp-admin/site-editor.php?postType=page&postId=99999, the page preview page is stuck on loading, and a 404 not found is thrown on the Network Tab of the developer console.

While on,

WP 6.5
Theme: Twenty Twenty-Three

It does not get stuck and the URL /wp-admin/site-editor.php?path=%2Fpage&canvas=view is configured and shown if we travel to /wp-admin/site-editor.php?postType=page&postId=999999, non-existent postId.

Added validation for the postId parameter in site-editor.php to ensure that the post exists before proceeding. If the post ID is invalid or the post does not exist, the script will now terminate with an error message, preventing potential errors or unintended behavior.

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

Though it's probably beyond the scope of the original ticket, I see a lot of duplication of isset( $_GET['postType'] ) and sanitize_key( $_GET['postType'] ).

Wondering if we could have something like a default $current_post_type = false|null then check and sanitize it once for reuse in the rest of the file. Same could pro be done for $_GET['postId'] and $_GET['path']?

Could probably move the early nails a bit higher in the file as well, to save some unnecessary code running when it's not needed? Just based off a brief look, unless I am missing something, I don't see anything between lines 76 and 118 that is in any way dependent on any of the code running above.

[iflairwebtechnologies]

@benniledl
you can review, created a new patch 61796.patch, which will also work for draft and trash post

would be nice to have an error message for various post statuses as in #7215

@dream-encode @peterwilsoncc Thank you for the suggestions. I have made the changes.

[hellofromTonya]

I can reproduce this with 6.6.2.

In comparison, 6.5.5 redirects to site-editor.php?path=%2Fpage&canvas=view.

Before 6.6.0, it redirected to site-editor.php?path=%2Fpage&canvas=view. Granted that did not indicate the _why_.

Why wp_die()?

My concern is two-fold:

• Before this bug: It redirected, but stayed in the Site Editor.
• Will the user understand how to get back to the editor, i.e. get out of this error page?

I'm wondering:

Could a lighter approach might be a better user experience? "lighter" meaning -> retain the previous behavior to keep the user in the Site Editor, while also displaying a message to inform the user why their requested page did not show.

@dream-encode @peterwilsoncc What do you think?

@hellofromtonya How about passing link_url and link_text to the function call to return to the site editor?

Untested:

wp_die(
   __( 'Invalid page ID.' ),
   '',
   array(
      'link_url' => admin_url( 'site-editor.php' ),
      'link_text' => 'Return to site editor'
   )
);

I think hitting these error messages will require some degree of messing around with the URLs so it's safe to assume a certain amount of advanced user knowledge.

[chaion07]

Thanks @benniledl for reporting this. We reviewed this Ticket during a recent bug-scrub session. We feel the need for testing the method suggested in Peter's most recent comment.

Props to @mrinal013 for offering to help test this as we wait for the test report.

Cheers!

[sayedulsayem]

I have tested and implemented @peterwilsoncc suggestion. It works. I also tweaked some conditions to make it more secure like I matched post type with post ID. I am including the patch in this ticket. Thanks

[sayedulsayem]

patch added from the suggestion

[mrinal013]

Tested 61796.2.patch
When go to /wp-admin/site-editor.php?postType=page&postId=999999, get a 404 Not Found page which says `Invalid post ID. Return to site editor'

@hellofromtonya How about passing link_url and link_text to the function call to return to the site editor?

Untested:

wp_die(
   __( 'Invalid page ID.' ),
   '',
   array(
      'link_url' => admin_url( 'site-editor.php' ),
      'link_text' => 'Return to site editor'
   )
);

I think hitting these error messages will require some degree of messing around with the URLs so it's safe to assume a certain amount of advanced user knowledge.

That works in my testing. It mitigates one of my concerns:

Will the user understand how to get back to the editor, i.e. get out of this error page?

Let's adding that to each of the new wp_die() instances.

[stoyangeorgiev]

Discussed at a bug-scrub. With some changes requested and and not addressed yet, Beta 3 in a few hours, will move this one for 6.8.

Props to @pratiklondhe

@costdev @hellofromtonya Thanks a lot for the feedback. I have implemented the requested changes.

Hi @mi5t4n, thanks for the ping!

My contributions to WordPress Core are currently on hold. I am therefore unable to perform a follow-up review at this time.

Please drop a comment on the ticket to ask if other contributors are able to perform a follow-up review.

[poena]

Hi

This has been resolved upstream in the Gutenberg plugin and should, unless something unforseen happens, be part of WordPress 6.8.
See ​https://github.com/WordPress/gutenberg/pull/62274

On WordPress 6.7.1 or 6.8-alpha-59664 with Gutenberg (current trunk) active, navigating to
/wp-admin/site-editor.php?postType=page&postId=999999
redirects you to
/wp-admin/site-editor.php?postId=999999&p=%2Fpage

At the top of the page preview, there is a warning notice saying:
You attempted to edit an item that doesn't exist. Perhaps it was deleted?

[poena]

Notice displayed by Gutenberg

[oglekler]

I am removing needs-testing until this will be merged into Core.

[Mamaduka]

It seems that we're discussing two potential UIs that can be displayed for a user here, but I'm not exactly sure which one this ticket is trying to solve.

Missing item UI paths:

1. The site editor has a postType query argument, which isn't a registered post and displays a wp_die message. Example URL: wp-admin/site-editor.php?postType=missing
2. The site editor has query arguments for an entity (postType and postId), but the entity is missing from the DB. The following message is displayed in the editor canvas: "You attempted to edit an item that doesn't exist. Perhaps it was deleted?"

Possible solutions:

1. We could move the condition into _wp_get_site_editor_redirection_url and do a more graceful redirect instead of wp_die.
2. This is a legitimate error/warning. The editor can't render content if it doesn't exist. Maybe we can add an action to the warning to take the user to the Dashboard or something similar.

[audrasjb]

As per today's bug scrub, we're moving it to 6.9 as beta 3 is tommorrow. @Mamaduka please feel free to move it back to 6.8 if it's on your plate for tomorrow's Gutenberg merges.

[Mamaduka]

The problem isn't specific to WP 6.8 and is more like a design decision than a bug, IMO. It's okay to punt to WP 6.9.

Trac ticket:

Before: When a user tries to access wp-admin/site-editor.php?postId= with an invalid postId → A blank page appears → User gets stuck.

After: When accessing wp-admin/site-editor.php?postId= with an invalid postId → A warning is shown (“No specific content was found to edit. Use the editor sidebar to navigate to different templates, pages, or patterns.”) + additional info + functional editor → User can continue working.
On loading i have show an error message:

[TimothyBlynJacobs]

Thanks for the patch! I think this could maybe use some design attention. It seems like we're referencing the editor sidebar, but there is no sidebar shown. Feels like we need to make this more friendly.

Additionally, at least in my testing in Playground, that notice is only appearing for a brief moment, before disappearing, and redirecting me to the homepage.

[welcher]

Reviewed in the 6.9 bug scrub today. We're 1 week from RC 1 and this doesn't seem likely to be ready. I'm going to move it to Future Release.