Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 19 months ago

#61519 closed defect (bug) (fixed)

Unable to pick a featured image due to JS error

Reported by: david.binda Owned by: desrosj
Priority: normal Milestone: 6.6
Component: Build/Test Tools Version: 6.6
Severity: normal Keywords: commit dev-reviewed
Cc: Focuses:

Description

After r58563 there is a JS error in the media picker which results in just the first single from the media library being loaded upon clicking to "Set featured image" in the post editor.

The JS error is as follows:

TypeError: t.get is not a function
    at n.comparator (media-models.min.js?ver=6.6-RC1:2:5566)
    at e.comparator (media-views.min.js?ver=6.6-RC1:2:7240)
    at Array.sort (<anonymous>)
    at n.sort (backbone.min.js?ver=1.5.0:2:11690)
    at n.set (backbone.min.js?ver=1.5.0:2:10286)
    at n.add (backbone.min.js?ver=1.5.0:2:8973)
    at n.validate (media-models.min.js?ver=6.6-RC1:2:2960)
    at n._validateHandler (media-models.min.js?ver=6.6-RC1:2:3953)
    at p (backbone.min.js?ver=1.5.0:2:3818)
    at f (backbone.min.js?ver=1.5.0:2:3497)

While I'm still not sure what the root cause is, it seems like reverting the mentioned changeset for the wp-includes/js/media-views.min.js file only fixes the issue.

Steps to reproduce:

  1. make sure the minified versions of the JS files are being loaded ( eg.: no SCRIPT_DEBUG )
  2. have more than 1 image uploaded in a media library (eg.: 2)
  3. go to wp-admin/post-new.php
  4. click the "Set featured image" in the right hand side panel (on the Post level)
  5. see that just a single image got loaded, observe the JS error in the console

Change History (11)

#1 @mukesh27
2 years ago

  • Component GeneralEditor
  • Keywords needs-patch added
  • Milestone Awaiting Review6.6

Thanks @davidbinda for raising. I also replicate same issue in RC1

Moving to 6.6

#2 @alshakero
2 years ago

  • Component EditorGeneral
  • Keywords needs-patch removed

Thanks for the ticket! I investigated this last night and most probably it stems from updating uglify-js here. The source code of media-models.js hasn't changed in the past 4 years, yet the minified file is slightly logically different.

I suspect the output code is not logically equivalent to the source. To see the minification issue, you can see the diff I created here. The left side is the new minified code, the right side is the old (pre-change set) minified code.

https://www.diffchecker.com/aeLriFRa/

There may be other minification issues that we missed.

#3 @david.binda
2 years ago

  • Component GeneralEditor
  • Keywords needs-patch added

#4 @desrosj
2 years ago

  • Component EditorBuild/Test Tools
  • Keywords needs-patch removed
  • Owner set to desrosj
  • Status newreviewing

#5 @desrosj
2 years ago

  • Keywords commit dev-feedback added
  • Status reviewingaccepted

Thanks everyone! This should be fixed by [58585], which is not showing here because of a typo in the commit message.

Could everyone test this against trunk to confirm it fixes the problem? I've also rebuilt the nightly so that you can test using the Beta Tester plugin as well.

Also marking for a second committer sign off to backport once we're confident with the amount of testing.

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


2 years ago

#7 follow-up: @jorbin
2 years ago

  • Keywords dev-reviewed added; dev-feedback removed

[58585] Looks good for backport.

For testing, I built the change checked:

  1. Uploading new images, was able to select different featured ones
  2. Loading an existing post, was able to select and change the featured image
  3. On a new post, was able to select any image from the media library

#8 @david.binda
2 years ago

I've tested the proposed patch on WordPress.com using the same steps as mentioned in the original ticket. The [58585] addresses the described issue. Thanks for working on it!

#9 in reply to: ↑ 7 @hellofromTonya
2 years ago

Replying to jorbin:

[58585] Looks good for backport.

For testing, I built the change checked:

  1. Uploading new images, was able to select different featured ones
  2. Loading an existing post, was able to select and change the featured image
  3. On a new post, was able to select any image from the media library

Tested before and after [58585].

I can reproduce the reported issue.
Using @jorbin's approach, I can also confirm [58585] resolves the issue.

I 2nd [58585] looks good for backport to the 6.5 branch.

#10 @desrosj
2 years ago

  • Resolutionfixed
  • Status acceptedclosed

In 58586:

Build/Test Tools: Revert uglify-js update.

This partially reverts [58563], which applied an update of the uglify-js devDependency from 3.17.4 to 3.18.0.

The 3.18.0 update is causing some JavaScript errors in the media-views.min.js file, so needs to be investigated further.

Reviewed by jorbin, hellofromTonya.
Merges [58585] to the 6.6 branch.

Props david.binda, mukesh27, alshakero, jorbin, hellofromTonya.
Fixes #61519.

#11 @desrosj
19 months ago

In 59768:

Build/Test Tools: Configure UglifyJS to preserve previous behavior.

As of UglifyJS >= 3.18.0, the default behavior is to process input as an ES module. This updates the relevant configurations to ensure the build process continues to use the previous behavior to avoid JavaScript errors in the minified versions of files.

Follow up to [58563], [58586], and [59509].

Props siliconforks, nataliat2004, poena, mai21, SergeyBiryukov.
Fixes #62767. See #61519, #62220.

Note: See TracTickets for help on using tickets.