WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#24094 closed defect (bug) (fixed)

"Blank" media item in "Insert Media" popup

Reported by: batmoo Owned by: markjaquith
Milestone: 3.6 Priority: high
Severity: normal Version: 3.6
Component: Media Keywords: has-patch dev-feedback
Focuses: Cc:

Description

Steps to reproduce:

  • Make sure your site has some media items uploaded
  • Goto Posts > Add New
  • Click on Add Media
  • Goto Insert Media > Media Library
  • The second media item in the list will be blank

Running 3.6-beta1-23990

Attachments (2)

blank-media-item.png (385.7 KB) - added by batmoo 8 years ago.
24094.diff (620 bytes) - added by kovshenin 8 years ago.

Download all attachments as: .zip

Change History (13)

#1 @SergeyBiryukov
8 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 3.6
  • Priority changed from normal to high

Per ticket:23830:6, introduced in [23893].

#2 @kovshenin
8 years ago

  • Cc kovshenin added

#3 @adamsilverstein
8 years ago

  • Cc adamsilverstein@… added

@kovshenin
8 years ago

#4 @kovshenin
8 years ago

  • Keywords has-patch dev-feedback added; needs-patch removed

Spent some time (again) poking this around today and came up with 24094.diff which I'm not sure is the right thing to do, so would appreciate some feedback.

It looks like before [23893], the media.model.Attachments.parse method is fired only once when sync is complete, but after the changeset, it's fired once for the collection sync, and then once again for each attachment that was fetched.

The closest reason I found is the following note in the Backbone's 0.9.9 changelog (although it works perfectly fine in 0.9.10):

After fetching a model or a collection, all defined parse functions will now be run. So fetching a collection and getting back new models could cause both the collection to parse the list, and then each model to be parsed in turn, if you have both functions defined.

If that's what's causing the multiple calls to parse, then removing it seems like an okay thing to do, since the individual attachments will still get their own parse method called.

#5 @adamsilverstein
8 years ago

tested 24094.diff​ - indeed removed the blank square from the insert media modal; bravo!

#6 @dllh
8 years ago

  • Cc daryl@… added

24094.diff removes the extra item for me as well, without apparent regression.

#7 @zoonini
8 years ago

  • Cc zoonini added

#8 @richardmtl
8 years ago

  • Cc richard@… added

#9 @markjaquith
8 years ago

  • Owner set to markjaquith
  • Resolution set to fixed
  • Status changed from new to closed

In 24110:

Fix the appearance of a blank media item due to [23893].

props kovshenin. fixes #24094.

#10 @garyc40
8 years ago

This fix creates a regression in #24753 and should be reviewed and/or reverted.

#11 @markjaquith
8 years ago

In 24771:

Fix a media regression in [24110] that could cause duplicate models.

Props garyc40. Fixes #24753. See #24094, #23830.

Note: See TracTickets for help on using tickets.