WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 9 days ago

#49178 new defect (bug)

Upload image issue after deleting image from edit image page

Reported by: rnitinb Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.3.2
Component: Media Keywords: has-patch
Focuses: Cc:

Description

Hello,

I have found an issue in the image upload process.

Steps to recreate it:
1) First, open upload image page from wp-admin
2) Upload any image and then go to "Edit more details" page of uploaded image
3) Then, delete that image so it will redirect you to an upload page with ?deleted=1 parameter in a query string like http://localhost/elements/wp-admin/upload.php?deleted=1

OR

3)Just add ?deleted=1 after upload.php page of upload image page like http://localhost/elements/wp-admin/upload.php?deleted=1
4) Then, upload a new image for show issue.

The issue is that, after deleting an image and if you upload an image, an image can not be displayed in the image gallery. You must refresh the page to see that image.

The issue is in the video => https://www.youtube.com/watch?v=_S6z3BySB64

Attachments (1)

49178.1.patch (8.7 KB) - added by Mista-Flo 2 weeks ago.

Download all attachments as: .zip

Change History (9)

#1 @SergeyBiryukov
8 months ago

  • Component changed from General to Media

This ticket was mentioned in Slack in #core-media by rnitinb. View the logs.


8 months ago

#3 @rnitinb
5 months ago

Hello,

I have submitted this issue 4 months ago but there is no update on this yet.

This issue still exists in WordPress 5.4.1, please take a look into this.

Thanks & regards.

This ticket was mentioned in Slack in #core by rnitinb. View the logs.


3 months ago

#5 @swissspidy
3 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

#6 @Mista-Flo
2 weeks ago

I'm looking at this issue.

First, before looking at the ticket issue, I have to say that the upload.php is a tough one. It handles the both list view and grid view in a very different way. The grid mode triggers a different markup at all, it loads a JS underscore template. One of the drawbacks with the current implementation IMO is that we don't display messages in grid view. In list view, if you do the same actions, you would have a notice just above the filters saying that the file has been permanently deleted. I don't see any reason why that's not the case either for grid view. The bad thing is that there is no easy way to just use the same code (about messages) for both context.

@Mista-Flo
2 weeks ago

#7 @Mista-Flo
2 weeks ago

  • Keywords has-patch added; needs-patch removed

Hi, I have submitted a patch that fixes the issue. I am not sure about the implementation though so let's give your feedback.

Here's the explanation of the patch :

The curent behaviour is: When a GET deleted param is present, in list mode, it directly removes it from the URL query args, and drop a message.

With my patch, it now displays messages even in grid mode, and also unset from the $_GET superglobal the deleted and other params like this that was avoiding new uploaded images to get displayed in the grid.

You can have a closer look to this code:

<?php
        $q = $_GET;
        // Let JS handle this.
        unset( $q['s'] );
        $vars   = wp_edit_attachments_query_vars( $q );
        $ignore = array( 'mode', 'post_type', 'post_status', 'posts_per_page' );
        foreach ( $vars as $key => $value ) {
                if ( ! $value || in_array( $key, $ignore, true ) ) {
                        unset( $vars[ $key ] );
                }
        }

It might be a better option to ignore deleted and other params set, but I am not sure about the consequences.

Last edited 2 weeks ago by Mista-Flo (previous) (diff)

This ticket was mentioned in Slack in #core-media by florian-tiar. View the logs.


9 days ago

Note: See TracTickets for help on using tickets.