#57612 closed defect (bug) (fixed)
Deprecate `wp-admin/media.php`
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.3 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Media | Keywords: | early has-patch commit add-to-field-guide |
Focuses: | administration | Cc: |
Description
Is it time to deprecate the old Edit Media screen? I think so.
Reasons for removing it from core.
- The file is not linked to from core.
- The
Edit image
button (while editing images) leads to a visually broken screen. - The
Insert into Post
button leads to a white screen. - The
Update Media
button makes you leave the screen, taking you back to the media library list.
The file was introduced in #6181/[7262]. Not sure when it was removed from media workflow.
See #57608 for reference. Props to @costdev for also investigating this.
Attachments (1)
Change History (42)
#2
@
2 years ago
Here are some references to usage by plugins and themes. There is only one plugin with installs that has been tested up to the latest three major releases, or within the last two years.
If there are concerns about backward compatibility that mean this file should not be deprecated, that's understandable.
Otherwise, this file has been patched many times over the past number of years adding to the maintenance burden, and we should consider deprecating it.
#3
in reply to:
↑ description
@
2 years ago
Replying to kebbet:
The file was introduced in #6181/[7262]. Not sure when it was removed from media workflow.
I think it was removed in [21948] / #21391, see the change to _edit_link
for the attachment
post type:
- '_edit_link' => 'media.php?attachment_id=%d', /* internal use only. don't use this when registering your own post type. */ + '_edit_link' => 'post.php?post=%d', /* internal use only. don't use this when registering your own post type.
This ticket was mentioned in PR #3979 on WordPress/wordpress-develop by @kebbet.
2 years ago
#4
- Keywords has-patch added; needs-patch removed
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
2 years ago
#7
@
2 years ago
- Keywords early added; 2nd-opinion removed
- Milestone changed from Awaiting Review to 6.3
This ticket was discussed in a recent Media meeting and it was agreed that this should move forward and deprecate the view. A couple things noted.
@joedolson mentioned "If there are any functions here that need to be kept, they should be moved to wp-includes, anyway."
Also it was discussed that after this is removed we put in place a redirect for this pattern to go to the attachment page of the intended media.
Adding 6.3 and early so we can try to get this in as soon as possible to determine the impact.
@audrasjb commented on PR #3979:
23 months ago
#8
I refreshed the PR to update the since
mention add for trunk merge purpose :)
23 months ago
#9
I refreshed the PR to update the
since
mention add for trunk merge purpose :)
Should the last line also be updated to use 6.3.0
?
@audrasjb commented on PR #3979:
23 months ago
#10
Ah yes, updated :)
#12
@
23 months ago
- Keywords commit added
- Owner set to audrasjb
- Status changed from new to accepted
Self assigning for a last review before commit.
@audrasjb commented on PR #3979:
23 months ago
#14
Committed in https://core.trac.wordpress.org/changeset/55647
#15
@
23 months ago
- Keywords needs-patch added; has-patch commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
As seen in the screen shot above, visiting media.php following [55647] results in a white screen without any user facing information. As a number of plugins reference the file, the most popular with 200K+ installs, I don't think this is adequate.
I think a redirect to admin_url( '/uploads.php' )
is more suitable. If placed after the deprecation notice, a headers_sent()
check will be required.
#16
@
23 months ago
I think we should redirect this page rather than having it white screen. I would suggest going to wp-admin/upload.php
ideally with a notice that the previous page was removed in 6.3
apparently, @peterwilsoncc had the exact same thought.
#17
@
23 months ago
You got it, and wp-admin/upload.php
looks like the best redirect. Having a notice displayed after the redirection whould indeed be ideal.
I can't see any precedent for such a redirection notice. Would a notice like this do the job?
It looks like the screen you are looking for does not exist. You have been redirected to this page instead.
(has to be rephrased, just putting words together)
And with a redirection using a query var to display the notice?
As I couldn't find any precedent, just wanted to make sure we're doing it right :)
#18
@
23 months ago
It could be an automatic redirect + a note, or perhaps a notice on wp-admin/media.php
with a link to upload.php
, i.e. info + manual redirect.
Seems both cases would need require_once __DIR__ . '/admin.php';
as it seems media.php was an entry point.
#19
@
23 months ago
And with a redirection using a query var to display the notice?
Yes. I would do it like that.
Riffing on your wording, The Edit Media screen is deprecated as of WordPress 6.3. Please use the Media Library instead
also, _deprecated_file
causes a 500 error since the function isn't defined if you go directly to wp-admin/media.php
. Seems to also be the case for wp-admin/custom-header.php
#20
@
23 months ago
Completely untested but how about something like the following code.
Unfortunately, upload.php
removes the wp_admin_canonical_url
action from the admin_init
screen (not sure why) so the error
would remain in the URL. It may be possible to reinstate it and use the removable_query_args
filter instead. -- This may be a terrible idea, too :)
<?php /** * Media management action handler. * * @package WordPress * @subpackage Administration */ /** Load WordPress Administration Bootstrap */ require_once __DIR__ . '/admin.php'; $parent_file = 'upload.php'; $submenu_file = 'upload.php'; wp_reset_vars( array( 'action' ) ); switch ( $action ) { case 'editattachment': case 'edit': // Used in the HTML title tag. $title = __( 'Edit Media' ); if ( empty( $errors ) ) { $errors = null; } if ( empty( $_GET['attachment_id'] ) ) { wp_redirect( admin_url( 'upload.php' ) ); exit; } $att_id = (int) $_GET['attachment_id']; wp_redirect( admin_url( "upload.php?item={$att_id}&error=throw-depreciated-media.php" ) ); exit; default: wp_redirect( admin_url( 'upload.php?error=throw-depreciated-media.php' ) ); exit; }
#21
@
23 months ago
Just to understand, will we be unable to e.g. crop/rename attachments after this or will these features be moved to another place?
Since thats what is unclear to me now. What features is this going to remove (regardless if they are buggy or not) and what replacements do we have/are needed?
#22
follow-up:
↓ 24
@
23 months ago
@NekoJonez Renaming, croping, editing etc. was moved to post.php in [21948], so no functionality is removed. What was removed in [55647] was the predecessor to the current workflow, which has beed broken for years, and is not accessible from any link in core.
@peterwilsoncc regarding the references to media.php in the 200000 installs plugin, the reference is only present in the *.pot-file of the plugin, no actual links. Same goes for all plugins above 5000 I looked at in the linked list (.po/.pot). The Media tags
plugin with 4000 is closed.
This ticket was mentioned in PR #4320 on WordPress/wordpress-develop by @kebbet.
23 months ago
#23
- Keywords has-patch added; needs-patch removed
https://core.trac.wordpress.org/ticket/57612
A follow up to https://core.trac.wordpress.org/changeset/55647.
Based on suggestions from @peterwilsoncc and @aaronjorbin
#24
in reply to:
↑ 22
@
23 months ago
Replying to kebbet:
@NekoJonez Renaming, croping, editing etc. was moved to post.php in [21948], so no functionality is removed. What was removed in [55647] was the predecessor to the current workflow, which has beed broken for years, and is not accessible from any link in core.
@peterwilsoncc regarding the references to media.php in the 200000 installs plugin, the reference is only present in the *.pot-file of the plugin, no actual links. Same goes for all plugins above 5000 I looked at in the linked list (.po/.pot). The
Media tags
plugin with 4000 is closed.
Thanks for the answer. Now, I'm going to put on my other hat as a polyglot.
Shouldn't these pot files be recreated then? So, translations still happen correctly?
#25
@
23 months ago
- Keywords changes-requested added
Added couple suggestions to https://github.com/WordPress/wordpress-develop/pull/4320.
#26
@
23 months ago
- Keywords changes-requested removed
Linked PR has been updated. Please have another look at it.
@peterwilsoncc commented on PR #4320:
22 months ago
#30
@kebbet The WordPress dashboard uses the JavaScript history API to remove querystring parameters that used to display one time messages/errors, it prevents the message showing if someone bookmarks the URL or copies and pastes it.
To see an example, visit /wp-admin/edit.php?trashed=1&ids=1
and you'll see the trashed message displayed but the querystring parameters will be removed via JavaScript.
As the error this PR is adding to the uploads screen is intended as a one time message, it would be good to make use of the feature to remove the error
parameter.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
21 months ago
21 months ago
#33
{{{diff
+ add_filter(
+ 'removable_query_args',
+ function( $query_args ) {
+ return array( 'error' );
+ }
+ );
}}}
@peterwilsoncc If I add this change to the file, there is an error in the editor, Symbol '$query_args' is declared but not used.
Is that expected?
@peterwilsoncc commented on PR #4320:
21 months ago
#34
there is an error in the editor,
Symbol '$query_args' is declared but not used.
Is that expected?
That's expected and a false report as the code replaces the value entirely.
21 months ago
#35
Ok, thanks for quick reply @peterwilsoncc !
I've updated the PR with the suggestion, with an addition of a one line comment, please review. And test.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
21 months ago
#37
@
20 months ago
- Keywords commit added
The linked pull request looks good for a follow-up commit.
@peterwilsoncc commented on PR #4320:
20 months ago
#39
#41
in reply to:
↑ 1
@
4 months ago
Replying to kebbet:
The screen is accessible via
wp-admin/media.php?attachment_id=NNN&action=edit
, where NNN is attachment id.
you could try instead either
wp-admin/upload.php?item="the image_id";
wp-admin/post.php?post="the image_id"&action=edit;
It worked for me.
The screen is accessible via
wp-admin/media.php?attachment_id=NNN&action=edit
, where NNN is attachment id.