Make WordPress Core

Opened 5 years ago

Closed 3 years ago

#47353 closed defect (bug) (fixed)

jquery.masonry.js Not Found

Reported by: pondermatic's profile pondermatic Owned by: desrosj's profile desrosj
Milestone: 5.8 Priority: normal
Severity: minor Version: 3.5
Component: Script Loader Keywords: has-patch commit
Focuses: Cc:

Description

When SCRIPT_DEBUG is true and the --dev option is used with grunt build , the un-minified version of jQuery Masonry is requested but not found.

Attachments (2)

47353.patch (415 bytes) - added by pondermatic 5 years ago.
47353.diff (949 bytes) - added by desrosj 3 years ago.

Download all attachments as: .zip

Change History (11)

@pondermatic
5 years ago

#1 @pondermatic
5 years ago

  • Keywords has-patch added

#2 @pondermatic
5 years ago

This is a followup to #45869.

#3 follow-up: @isabel_brison
3 years ago

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

Thanks for the patch @pondermatic! However, the issue has since been fixed: https://github.com/WordPress/wordpress-develop/blob/master/Gruntfile.js#L211, so I'm closing this ticket.

#4 in reply to: ↑ 3 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 5.8
  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to isabel_brison:

However, the issue has since been fixed: https://github.com/WordPress/wordpress-develop/blob/master/Gruntfile.js#L211

Hmm, the line that the patch removes is still there, so it's not clear to me why or how this is fixed :)

Reopening for a closer review.

#5 @isabel_brison
3 years ago

Hmm, the line that the patch removes is still there, so it's not clear to me why or how this is fixed :)

Whoops, that's right! Not sure how I jumped to that conclusion :/

So I tested and the patch applies cleanly and works correctly. It can be checked by enabling Twenty Fourteen, and then adding a widget to the Footer Widget Area in the Customizer. Without the patch, jquery.masonry.js returns a 301. With the patch, it returns a 200.

One thing I noticed is this unminified script is requested whatever the value of SCRIPT_DEBUG; instead it seems to depend on whether $wp_version has -src. It may be tangential to this ticket, but I'm wondering why that's the case.

#6 follow-up: @azaozz
3 years ago

  • Keywords has-patch removed

The proper fix seems to be to fix the URL in script-loader.php. WP doesn't include non-minified version of jquery-masonry (the Masonry v2 shim) in the build files. Looking at Gruntfile.js not including it seems to have been a decision, not an error. Fix would be to replace $dev_suffix with .min at https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/script-loader.php#L842.

Last edited 3 years ago by azaozz (previous) (diff)

#7 in reply to: ↑ 6 @desrosj
3 years ago

  • Keywords has-patch commit added

Replying to azaozz:

The proper fix seems to be to fix the URL in script-loader.php. WP doesn't include non-minified version of jquery-masonry (the Masonry v2 shim) in the build files.

I agree with this. Because the v2 release is only included as a courtesy (developers should actually switch to using the masonry script handle instead), hard coding .min would fix this error without adding another unminified file to the release package.

There is also no unminified version of the masonry script handle either. But, this is a somewhat old library (no updates in 3 years), so I don't think we should add it now.

@desrosj
3 years ago

#8 @desrosj
3 years ago

  • Component changed from Build/Test Tools to Script Loader
  • Version changed from 5.2.1 to 3.5

Introduced in [22228] as part of #20346.

#9 @desrosj
3 years ago

  • Owner set to desrosj
  • Resolution set to fixed
  • Status changed from reopened to closed

In 51094:

Script Loader: Fix 404 errors for the jquery-masonry script when SCRIPT_DEBUG is true.

The unminified version of jquery-masonry has never been included in Core. This hard codes the .min into the file source to prevent these errors.

Props pondermatic, isabel_brison, SergeyBiryukov, azaozz, desrosj.
Fixes #47353.

Note: See TracTickets for help on using tickets.