Make WordPress Core

Opened 7 years ago

Closed 17 months ago

Last modified 2 months ago

#42760 closed defect (bug) (worksforme)

Trying to access an attachment directly by URL not working

Reported by: ppetrov2c's profile ppetrov2c Owned by: adamsilverstein's profile adamsilverstein
Milestone: Priority: normal
Severity: normal Version: 4.9.1
Component: Media Keywords: has-patch needs-testing
Focuses: javascript Cc:

Description

For example the URL https://example.com/wp-admin/upload.php?item=14
If there are more than 40 attachments, trying to access any attachment page after that 40th entry, will result in a JS error:

Uncaught TypeError: Cannot read property 'set' of undefined

Trying to open any attachment page via URL before and including 40, will work as expected.

A quick look, and it appears that, the problem comes from media-grid JS.

item = library.findWhere( { id: parseInt( query, 10 ) } );

The line above will return undefined, resulting in the attachment page not showing.

I'm attaching a screenshot with a log of the library variable from the media-grid JS file (line 521).

https://i.imgur.com/euFKa0p.png

Attachments (1)

42760.diff (762 bytes) - added by adamsilverstein 5 years ago.

Download all attachments as: .zip

Change History (14)

#1 follow-up: @adamsilverstein
7 years ago

  • Owner set to adamsilverstein
  • Status changed from new to reviewing

@ppetrov2c Thanks for the bug report and welcome to WordPress trac!

Testing locally I was not able to reproduce this issue. Can you please try disabling all plugins and also switching to one of the default themes (twentyseventeen for example) to see if the problem persists?

#2 @adamsilverstein
7 years ago

  • Focuses javascript added

#3 in reply to: ↑ 1 @ppetrov2c
7 years ago

Replying to adamsilverstein:

@ppetrov2c Thanks for the bug report and welcome to WordPress trac!

Testing locally I was not able to reproduce this issue. Can you please try disabling all plugins and also switching to one of the default themes (twentyseventeen for example) to see if the problem persists?

Forgot to mention, sorry about that - I did the tests on vanilla WordPress 4.9.1, without plugins and the twentyseventeen theme.

#4 @adamsilverstein
5 years ago

  • Keywords has-patch reporter-feedback added
  • Milestone changed from Awaiting Review to 5.4
  • Status changed from reviewing to accepted

In 42760.diff :

Media: ensure item is available before setting skipHistory on load.

@ppetrov2c Thanks for your bug report! I finally was able to work on looking into the issue. Can you please test the attached patch to verify this resolves the issue for you?

Anyone else who wants to test, its important that you:

  1. Have more than 40 items in your media library.
  2. Click on the 41st or greater item in the media library.
  3. Reload the browser window.

Before patch: Media item does not load, library shows instead, JavaScript console error
After patch: Item loads, no errors.

#5 @adamsilverstein
5 years ago

  • Keywords needs-testing added

#6 @audrasjb
5 years ago

  • Milestone changed from 5.4 to Future Release

Hi,

With 5.4 Beta 3 approaching and the Beta period reserved for bugs introduced during the cycle, this is being moved to Future Release. If any maintainer or committer feels this should be included or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#7 @ppetrov2c
4 years ago

@adamsilverstein - hey!

I apologize for the late reply. I managed to test the patch on the same test instance (clean wp 4.9.1, twentyseventeen).

I am happy to report, that the problem is no longer present. The 41st (and so on) item are loading after a browser refresh.

Decided to check if there's a problem in the latest WP (5.8) and it's not.

Version 0, edited 4 years ago by ppetrov2c (next)

#8 @ppetrov2c
4 years ago

  • Keywords reporter-feedback removed

#9 @adamsilverstein
4 years ago

Thanks for checking @ppetrov2c.

Would be great if we had test coverage for this type of thing as well (which I'm not sure we can given our current testing infrastructure)

Decided to check if there's a problem in the latest WP (5.8) and there isn't.

I wonder if that is true when users re-enable infinite scroll? If so, might still be worth fixing, I'll give it a test.

Last edited 4 years ago by adamsilverstein (previous) (diff)

#10 @adamsilverstein
4 years ago

  • Milestone changed from Future Release to 5.9

#11 @adamsilverstein
4 years ago

  • Milestone changed from 5.9 to Future Release

#12 @joedolson
17 months ago

  • Milestone Future Release deleted
  • Resolution set to worksforme
  • Status changed from accepted to closed

Just tested this with a few different cases: with infinite scroll re-enabled and default, tested the 41st+ item and also the 81st+ item, since the default collection size was changed in [51643].

In all cases, this appeared to work just fine, matching the test results that @ppetrov2c was observing in comment 7. Closing as worksforme. Feel free to reopen if you're experiencing this error!

@desrosj commented on PR #127:


2 months ago
#13

Closed as worksforme in Trac.

Note: See TracTickets for help on using tickets.