Media: Grid View: new upload, file is in the wrong position in the grid until after upload is complete

[lancewillett]

Steps to reproduce

1. Log in to your WordPress site.
2. Go to wp-admin/upload.php (or click Media link in the wp-admin menu)
3. Take a look at the grid view of images and files, if you have any there.
4. Now, add a new file.
5. Pay attention to where the new file goes in the grid during the upload. This is more obvious on a slow host with a huge file like a 1GB video.

What I expected
I expected the new file to appear at the top left position in the grid during upload and processsing.

What happened instead
The file appears inside the grid, not in the position I expected. The result is — as an author — that I feel like the upload didn't work because my eyes are looking for the new placeholder at the top left of the grid.

Note: the upload does work. And it does appear at the position I expect — but only after upload is complete and the file is saved.

Browser / OS version
Tested on OS X 10.12.6 with Safari, Chrome, and Firefox (all latest stable).

Screenshot / Video
​https://cloudup.com/cHuVsO0OQd1 -- animated GIF

Attaching 2 screenshots: step 1, you'll see the new image file is in-between the 2 existing files. After upload, it moves to the left as expected.

Context / Source
Real life usage on WordPress.com; tracked it back to a vanilla WP install, version 5.0-alpha-42125-src. Two plugins: Debug Bar and Debug Bar console. Default theme.

[lancewillett]

Step 1 of upload

[lancewillett]

Step 2 of upload

[lancewillett]

Animated GIF to show the bug as I upload a large video file

[designsimply]

Tested and confirmed that pending uploads in the Media Library appear in the 2nd to last position during upload instead of the top left position as expected. I also found that if you upload multiple images, they appear as duplicates until the entire set has completed uploading.

Video: ​1m58s

^{Seen at /wp-admin/upload.php using WordPress 4.9.1, Safari 11.0.1, macOS 10.13.2.}

[adamsilverstein]

@lancewillett Thanks for the excellent bug report.

Using git bisect, I tracked this down: it was introduced in r41937 (​on github)

[adamsilverstein]

cc: @joemcgill, @westonruter

[dd32]

Bumping to 4.9.3 due to 4.9.2s release

[adamsilverstein]

In 42968.diff​:

• remove call to this._filterContext when a model is added.

This corrects the reported issue in my testing. I don't see any side effects, however I'm not certain what this line of code is intended to do - appreciate any feedback/help from @joemcgill or @westonruter.

@lancewillett can you confirm this patch fixes the issue for you?

[lancewillett]

Yes, I can confirm the patch fixes the issue (running latest trunk). With the patch applied, the file upload is correctly positioned as expected.

[Junaidkbr]

I can confirm too. Latest trunk, patch applied. Works as expected, new processing upload is placed as first item.

[dd32]

Bumping, 4.9.4 has been released.

[adamsilverstein]

I spent some time reviewing #21819 where this issue was introduced. The purpose of the filtering was to "Filter out contextually created attachments (e.g. headers, logos, etc.)". My previous proposed patch broke that.

In 42968.3.diff​ I take a different approach, preventing these items from ever being added to the collection in Attachments.validator which is called by Attachments.validate. This prevents the resort caused by the previous approach and the misplaced imaged we were seeing.

@Junaidkbr @lancewillett can you give this new patch another test?

[Junaidkbr]

Tested on a clean build. The new patch works and resolves the initial issue.

[lancewillett]

Confirmed 42968.3.diff also fixes the issue, with latest trunk. Looks good.

[adamsilverstein]

@joemcgill could you take a look at this and confirm this still fixes the original issue?

[joemcgill]

@adamsilverstein this approach seems much cleaner than my previous attempt and I can confirm that it works as intended. Nice one!

[adamsilverstein]

Great, thanks for testing! Let's get this in so it can get some wider testing.

[adamsilverstein]

#43337 was marked as a duplicate.

[adamsilverstein]

In 42739:

Media: grid view - correct placeholder positioning during uploads.

Preventing contextually created attachments from being added to the grid collection in Attachments.validator. Remove the previous filtering introduced in [41937] which caused the placement issue.

Props lancewillett, Junaidkbr, designsimply, joemcgill.
Fixes #42968.

[adamsilverstein]

Reopening for consideration for the 4.9.5 branch.

[adamsilverstein]

Amended in [42740] removing a trailing comma to address a jshint error.

[ocean90]

In 42848:

Media: Grid view - correct placeholder positioning during uploads.

Preventing contextually created attachments from being added to the grid collection in Attachments.validator. Remove the previous filtering introduced in [41937] which caused the placement issue.

Merge of [42739-42740] to the 4.9 branch.

Props lancewillett, Junaidkbr, designsimply, joemcgill.
Fixes #42968.