Opened 6 months ago
Last modified 2 weeks ago
#61796 new defect (bug)
Blank Page When $_GET['postId'] Does Not Exist in /wp-admin/site-editor.php
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | 6.8 | Priority: | normal |
Severity: | normal | Version: | 6.6 |
Component: | Editor | Keywords: | has-patch changes-requested gutenberg-merge |
Focuses: | Cc: |
Description
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.
Attachments (5)
Change History (29)
This ticket was mentioned in PR #7188 on WordPress/wordpress-develop by @mi5t4n.
6 months ago
#2
- Keywords has-patch added
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
@davidbaumwald commented on PR #7188:
6 months ago
#3
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.
#4
@
6 months ago
@benniledl
you can review, created a new patch 61796.patch, which will also work for draft and trash post
This ticket was mentioned in PR #7215 on WordPress/wordpress-develop by @iflairwebtechnologies.
6 months ago
#5
@benniledl commented on PR #7188:
6 months ago
#6
would be nice to have an error message for various post statuses as in #7215
6 months ago
#7
@dream-encode @peterwilsoncc Thank you for the suggestions. I have made the changes.
#8
@
4 months ago
- Keywords changes-requested needs-testing added
- Milestone changed from Awaiting Review to 6.6.3
I can reproduce this with 6.6.2.
In comparison, 6.5.5 redirects to site-editor.php?path=%2Fpage&canvas=view
.
@hellofromTonya commented on PR #7188:
4 months ago
#9
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?
@peterwilsoncc commented on PR #7188:
4 months ago
#10
@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.
This ticket was mentioned in Slack in #core by chaion07. View the logs.
4 months ago
#12
@
4 months ago
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!
#13
@
4 months ago
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
This ticket was mentioned in PR #7520 on WordPress/wordpress-develop by @sayedulsayem.
4 months ago
#14
This ticket was mentioned in PR #7521 on WordPress/wordpress-develop by @sayedulsayem.
4 months ago
#15
#16
@
4 months ago
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 commented on PR #7188:
4 months ago
#17
@hellofromtonya How about passing
link_url
andlink_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.
This ticket was mentioned in Slack in #core by stoyangeorgiev. View the logs.
4 months ago
#19
@
4 months ago
- Milestone changed from 6.6.3 to 6.8
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
2 months ago
#20
@costdev @hellofromtonya Thanks a lot for the feedback. I have implemented the requested changes.
2 months ago
#21
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.
#22
@
3 weeks ago
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?
Hello,
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,
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.