Make WordPress Core

Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#53419 closed defect (bug) (fixed)

Media: Links to individual media items don't work

Reported by: ryelle Owned by: joedolson
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch commit
Focuses: javascript Cc:


It looks like links to individual media items have stopped working after r51145/#50105.

The expected behavior, on beta1:

  • Go to Media, make sure you're in grid view
  • Click a media
  • The attachment modal opens, and the URL updated to something like /wp-admin/upload.php?item=7
  • You can reload the page, the URL stays the same, and the same attachment modal opens

In trunk or beta2:

  • The attachment modal opens on click, but does not change the URL.
  • Going directly to /wp-admin/upload.php?item=7 does not open the attachment.

Attachments (2)

53419.diff (2.0 KB) - added by adamsilverstein 4 months ago.
53419.2.diff (2.0 KB) - added by adamsilverstein 4 months ago.
resolve a linter issue

Download all attachments as: .zip

Change History (13)

This ticket was mentioned in PR #1381 on WordPress/wordpress-develop by adamsilverstein.

4 months ago

  • Keywords has-patch added

#2 @adamsilverstein
4 months ago

Hey @ryelle :} thanks for catching and raising this issue, and especially for identifying the offending commit!

This is caused by our changing the deferred resolution to use the (typical) deferred.jqXHR instead of core's setting of this which causes us to not have access to the response headers in the collection's parse method (in attachments.js).

This tricky part is we need access to both the collection and the request jqxhr to pull the collection totals from the header response and set them in the collection.

In 53419.diff I moved the logic out of the custom fetch method we added into wp.ajax.send where we have access to both the collection and the actual xhr response. The only addition to the existing code is a check to make sure this is a media request and capturing the context (the attachments collection) before the done handler so we can set the attachment count.

Can you please give it a test and confirm this corrects the issue you were seeing? It does in my testing.

#3 @adamsilverstein
4 months ago

  • Keywords reporter-feedback added

4 months ago

resolve a linter issue

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

4 months ago

#5 @antpb
4 months ago

I was able to reproduce the issue on a site running nightly. I applied the patch to my local and it seems to restore the behavior of appending the item id on click allowing a refresh to re-open the modal.

Looks good to me, but I'm happy to wait for reporter-feedback.

#6 @ryelle
4 months ago

  • Keywords reporter-feedback removed

Yes, this fixes the problem, thanks!

#7 @audrasjb
4 months ago

I also experienced this issue and I was going to report it as well.
I'm not going to add commit keyword as I'm not a media library specialist but for what it's worth, the patch fixes the issue on my side. Thank you!

#8 @joedolson
4 months ago

  • Owner set to joedolson
  • Status changed from new to accepted

#9 @joedolson
4 months ago

  • Keywords commit added

#10 @joedolson
4 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 51187:

Media: Adapt response shape depending on type of query.

Restore inheriting the backbone fetch in the media library and adapt the AJAX response according to the action performed in the media query.

In [51145], the response shape was restored to the original shape, and a custom fetch was added to handle assigning the totalAttachments information in the collection. The custom fetch triggered a new set of bugs relating to zero-sized collections and loading individual images.

props adamsilverstein, ryelle, peterwilsoncc, Presskopp, desrosj.
Fixes #53421, #53419.

#11 @desrosj
4 months ago

In 51238:

Widgets: Avoid a TypeError when adding a widget in the Customizer.

This fixes a bug where action is set to null when adding a widget in the Customizer.

Fixes #53488. See #53421, #53419.

Note: See TracTickets for help on using tickets.