Deprecate `wp-admin/media.php`

[kebbet]

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.

[kebbet]

The screen is accessible via wp-admin/media.php?attachment_id=NNN&action=edit, where NNN is attachment id.

[costdev]

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.

• ​Plugins
• ​Themes

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.

[SergeyBiryukov]

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.

https://core.trac.wordpress.org/ticket/57612

[antpb]

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.

I refreshed the PR to update the since mention add for trunk merge purpose :)

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 ?

Ah yes, updated :)

[kebbet]

6.3 early is now, any chance the linked PR can be committed anytime soon?

[audrasjb]

Self assigning for a last review before commit.

[audrasjb]

In 55647:

Media: Deprecate wp-admin/media.php.

The wp-admin/media.php file was introduced in [7262], then removed from the Media workflow in [21948].
This changeset finally deprecates it as it is not used anymore.

Follow-up to [7262], [21948].

Props kebbet, costdev, SergeyBiryukov, jrf, antpb, audrasjb.
Fixes #57612.
See #6181, #21391, #57608.

Committed in https://core.trac.wordpress.org/changeset/55647

[peterwilsoncc]

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.

[jorbin]

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.

[audrasjb]

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 :)

[azaozz]

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.

[jorbin]

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

[peterwilsoncc]

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;
}

[NekoJonez]

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?

[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.

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

[NekoJonez]

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?

[azaozz]

Added couple suggestions to ​https://github.com/WordPress/wordpress-develop/pull/4320.

[kebbet]

Linked PR has been updated. Please have another look at it.

{{{diff
return array( 'error' );
}}}

Whats the idea behind it?

{{{diff
return array( 'error' );
}}}

Whats the idea behind it?

What's the idea behind the suggested addition to wp-admin/uploads.php?

@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.

Thanks Colin for a review, adjusted the PR to align with your suggestion!

{{{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?

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.

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.

[peterwilsoncc]

The ​linked pull request looks good for a follow-up commit.

[peterwilsoncc]

In 55943:

Media: Redirect deprecated wp-admin/media.php file.

Redirect users visiting the wp-admin/media.php file to the media library, wp-admin/upload.php. An user facing warning is displayed when the media library is reached via a deprecated link.

Follow up to [55647].

Props jorbin, audrasjb, azaozz, NekoJonez, kebbet, costdev.
Fixes #57612.

Committed in r55943 / ​https://github.com/WordPress/wordpress-develop/commit/6966e3193460a6ebd06916d6e188a02dbf31dde0