Opened 3 weeks ago
Last modified 4 hours ago
#65515 new defect (bug)
ThickBox ReferenceError: imgLoader is not defined due to "use strict" contamination in load-scripts.php
| Reported by: | bvedgie | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.0.2 |
| Component: | Script Loader | Version: | 7.0 |
| Severity: | normal | Keywords: | has-patch has-unit-tests needs-testing |
| Cc: | Focuses: | javascript |
Description
In WordPress 7.0, clicking on "View details" for any plugin in the admin dashboard fails to open the ThickBox modal window. The dark overlay (#TB_overlay) appears, but the modal content window (#TB_window) is never generated, or the loading spinner hangs indefinitely.
The browser console throws a fatal JavaScript error pointing to load-scripts.php.
Environment:
WordPress Version: 7.0
Scripts involved:
load-scripts.php?c=0&load%5Bchunk_0%5D=wp-hooks,jquery-core,jquery-migrate,thickbox&ver=7.0
Context: WordPress Admin Dashboard (Plugins page)
Console Error Log:
jQuery.Deferred exception: imgLoader is not defined ReferenceError: imgLoader is not defined
at HTMLDocument.<anonymous> (https://example.com/wp-admin/load-scripts.php?c=0&load%5Bchunk_0%5D=wp-hooks,jquery-core,jquery-migrate,thickbox&ver=7.0:25:12)
at e (https://example.com/wp-admin/load-scripts.php?c=0&load%5Bchunk_0%5D=wp-hooks,jquery-core,jquery-migrate,thickbox&ver=7.0:4:27028)
at t (https://example.com/wp-admin/load-scripts.php?c=0&load%5Bchunk_0%5D=wp-hooks,jquery-core,jquery-migrate,thickbox&ver=7.0:4:27330) undefined
Uncaught ReferenceError: imgLoader is not defined
at HTMLDocument.<anonymous> (load-scripts.php?c=0&load%5Bchunk_0%5D=wp-hooks,jquery-core,jquery-migrate,thickbox&ver=7.0:25:12)
Root Cause Analysis:
In wp-includes/js/thickbox/thickbox.js, the variable imgLoader is declared implicitly without any keyword (e.g., imgLoader = new Image();), which instantiates it as a global variable.
In WordPress 7.0, modern core scripts bundled in the same chunk (such as wp-hooks or jquery-core) enforce JavaScript's "use strict"; mode.
When load-scripts.php concatenates these scripts into a single file, the "use strict"; directive scope spills over, forcing the browser to evaluate the legacy thickbox.js code under Strict Mode rules.
Under Strict Mode, implicit global variable declaration is a fatal error, throwing the ReferenceError: imgLoader is not defined and crashing the execution queue before the modal can render.
Steps to Reproduce:
- Ensure script concatenation is enabled (default behavior).
- Go to the Plugins > Installed Plugins page.
- Click on the "View details" link of any plugin.
- Observe the screen freezing on the grey overlay and check the browser Console for the ReferenceError.
Temporary Workaround:
Adding define( 'CONCATENATE_SCRIPTS', false ); to wp-config.php resolves the issue. This forces WordPress to load scripts in separate <script> tags, isolating thickbox.js from the strict mode directive of other core files.
Proposed Fix:
Explicitly declare the variable with a proper keyword inside wp-includes/js/thickbox/thickbox.js to ensure Strict Mode compliance like adding "var" before "imgLoader = new Image".
(Note: cannot write code as it prevent ticket creation.)
Alternatively, ensure that load-scripts.php properly encapsulates concatenated files within self-invoking functions (IIFE) to prevent "use strict"; directives from bleeding into legacy, non-strict dependencies.
Change History (25)
#2
in reply to: ↑ 1
@
3 weeks ago
Replying to westonruter:
Is this truly a new issue in 7.0?
It looks like something did indeed change in 7.0. Compare these two versions:
https://github.com/WordPress/WordPress/blob/6.9.4/wp-includes/js/dist/hooks.js
https://github.com/WordPress/WordPress/blob/7.0/wp-includes/js/dist/hooks.js
The 7.0 version has "use strict" outside any function.
This ticket was mentioned in PR #12271 on WordPress/wordpress-develop by @khokansardar.
3 weeks ago
#3
- Keywords has-patch added; needs-patch removed
Fixes the WordPress 7.0 regression where ThickBox modals fail to open (e.g. "View details" on the Plugins screen), with the console error ReferenceError: imgLoader is not defined.
What the problem was:
- Since the esbuild build switch (changeset 22294 / commit 22294af4ed), wp-includes/js/dist/hooks.js begins with a top-level
"use strict";. - load-scripts.php concatenates requested scripts head-to-tail with
wp-hooksfirst, so that directive becomes the directive prologue for the entire concatenated file, forcing strict mode on every script after it. - Legacy non-strict scripts like ThickBox assign implicit globals (
imgLoader,imgPreloader,TB_WIDTH,ajaxContentW, ...), which throw ReferenceError in strict mode and abort the modal.
What the fix does:
- Begins the concatenated output with an empty statement (
$out = ";\n";), so a leading"use strict";is no longer the first statement and is evaluated as a harmless expression instead of enabling strict mode for the whole file.
Approach and why:
- The root cause is strict-mode contamination at the concatenation layer, not a single thickbox.js variable. Fixing only
imgLoaderwould move the crash to the next implicit global on the same code path and leave every other bundled legacy script exposed. - This one-line change restores the pre-7.0 behaviour (concatenated admin scripts ran in sloppy mode) and fixes all affected scripts at once.
- It is a no-op when CONCATENATE_SCRIPTS is disabled, since files are then served standalone; that path already runs hooks.js in strict mode, confirming no shipped script relies on strict mode for correctness.
Trac ticket: https://core.trac.wordpress.org/ticket/65515
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.8
Used for: Ticket analysis, regression bisection and root-cause verification. All changes were reviewed, validated against the codebase and verified with runtime reproduction, and are taken responsibility for by me.
#4
@
3 weeks ago
- Milestone Awaiting Review → 7.0.1
Further history:
- Commit 22294af is also known as [61438] (see build revision 60750).
- Gutenberg plugin changed the script in 21.9.0, likely related to GB72125.
#5
follow-up:
↓ 7
@
2 weeks ago
In addition to prepending the concatenated JS file with an empty expression to thwart any "use strict" statement, each script could be wrapped in an IIFE. However, the latter has the potential to break scripts worse because any var statements would then no longer be in the global scope.
So prepending the concatenated JS with an empty statement seems like the only option for now. I think a 0; as a prologue is slightly better than a ; since a JS optimizer could maybe strip out semicolons and it could be flagged as a linting problem. A comment could be added as well to the JS output:
0; /* prevent strict mode */
#6
follow-up:
↓ 11
@
2 weeks ago
- Milestone 7.0.1 → 7.0.2
As the RC for WP 7.0.1 is expected to land tomorrow we'll have to punt this to 7.0.2.
#7
in reply to: ↑ 5
;
follow-up:
↓ 8
@
2 weeks ago
Note that if a script that expects to be in strict mode, placing it in non-strict mode can break things.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode#semantic_differences
Replying to westonruter:
So prepending the concatenated JS with an empty statement seems like the only option for now.
Another option is just implementing #57548.
Yet another option is for Gutenberg to revert to the old build system. But it might not make much sense to do that if the plan is to implement #57548 anyway.
#8
in reply to: ↑ 7
@
2 weeks ago
Replying to siliconforks:
Note that if a script that expects to be in strict mode, placing it in non-strict mode can break things.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode#semantic_differences
Thanks for looking that up. I had suspected that.
Replying to westonruter:
So prepending the concatenated JS with an empty statement seems like the only option for now.
Another option is just implementing #57548.
Yet another option is for Gutenberg to revert to the old build system. But it might not make much sense to do that if the plan is to implement #57548 anyway.
I agree that implementing #57548 is the right way to go.
#9
@
2 weeks ago
- Keywords needs-patch added; has-patch removed
To fix this issue in the mean time, I think we should just fix the strict mode issues with thickbox.js.
I think this most straightforwardly looks like declaring all the variables at the top of the file, like:
var imgLoader, TB_PrevCaption, TB_PrevURL, TB_PrevHTML, /* ... and many more ... */
Not all of the variables that are currently getting added to the global scope are supposed to be so, it seems, however. So some judgement should be applied as to what is expected to be in the global scope.
This ticket was mentioned in PR #12371 on WordPress/wordpress-develop by @jorbin.
2 weeks ago
#10
- Keywords has-patch has-unit-tests added; needs-patch removed
Each of the variables declared at the top are included either are already used in the global scope or are prefixed.
Trac Ticket: https://core.trac.wordpress.org/ticket/65515
## Use of AI Tools
AI assistance: Yes
Debugged and initial patch including tests generated with Opencode and gpt-5.5. Manually checked and adjusted to reduce the number of variables in the global scope.
#11
in reply to: ↑ 6
@
2 weeks ago
- Milestone 7.0.2 → 7.0.1
Replying to masteradhoc:
As the RC for WP 7.0.1 is expected to land tomorrow we'll have to punt this to 7.0.2.
I think this would be good to get in 7.0.1 if we can get the testing and reviews necessary. I've uploaded a patch and am putting it back in the milestone but we can
@westonruter commented on PR #12371:
2 weeks ago
#12
With 0b5ea7e the file is now clean of JSHint errors (well, it is suppressing errors in what appear to be unrelated to strict mode or which are not actual issues).
@westonruter commented on PR #12371:
2 weeks ago
#13
I had Claude Opus 4.8 (with 1M context) search Veloria for the variables newly removed from the global scope along with the ones which are now explicitly made global:
The evidence is now conclusive. The nextgen-gallery match proves the pattern definitively: every hit for the thickbox-specific identifiers is inside a self-contained bundled copy of thickbox.js (static/Lightbox/thickbox/thickbox.js, containing its own tb_show, tb_init, var tb_pathToImage, etc.) — not external code reaching into WordPress core's globals.
## Findings
### 1. Variables now restricted to lexical scope (the breakage-risk set)
| Variable | New scope | External dependency found? |
|---|---|---|
imgPreloader | var (block) | No — but heavy false-positive name |
ajaxContentW / ajaxContentH | var (function) | No |
urlNoQuery | var (block) | No |
arrayPageSize | removed | No |
imgPreloaderhas huge raw counts (988 plugins / 667 themes), but it's a generic identifier used by many unrelated lightbox libraries in their own closures — e.g. WooCommerce's hit is injquery.prettyPhoto.js, which declares and uses its ownimgPreloader. None of these read WordPress core's value.ajaxContentW/H,urlNoQuery,arrayPageSizeare thickbox-specific, but every match lives inside a plugin/theme that ships its own full copy or fork of thickbox.js (the "ThickBox" plugin, "Responsive Thickbox", NextGEN, FunnelKit, buddyforms, Carrington theme, etc.). The per-extension match counts mirror the number of occurrences within thickbox itself — a self-contained copy, not a cross-reference.
These four were always function-internal temporaries, never part of any documented or de-facto API. Scoping them lexically (or removing arrayPageSize) is safe — nothing external could have been reading them off window.
### 2. Variables kept explicitly in global scope (the top-level var block)
imgLoader, TB_PrevCaption, TB_PrevURL, TB_PrevHTML, TB_NextCaption, TB_NextURL, TB_NextHTML, TB_imageCount, TB_FoundURL, TB_TempArray, TB_Counter, TB_WIDTH, TB_HEIGHT — plus the conditionally-declared tb_pathToImage.
- All external matches (
TB_WIDTH: 168/43 plugins, etc.) are again bundled thickbox copies/forks, not consumers of core's globals. So strictly speaking, even these didn't *need* to stay global for third-party compatibility. - The one variable with a genuine external contract is
tb_pathToImage: core thickbox reads it viaif ( typeof tb_pathToImage != 'string' )so a theme/plugin can pre-define the loading spinner before the script runs. The PR correctly keeps it as a top-levelvar(a window property in classic-script scope), so that override path still works.
## Bottom line
The PR's split is sound and backward-compatible:
- The variables it moved to lexical scope were internal scratch variables with zero external consumers — confirmed across the full plugin and theme corpus.
- The variables it kept global preserve the (mostly theoretical) global contract anyway, and critically retain the real
tb_pathToImageoverride hook.
No plugin or theme depends on WordPress core's thickbox leaking these as globals — the third-party matches are all carrying their own copies, which are unaffected by changes to core's file.
#14
@
2 weeks ago
For what it's worth, I can't reproduce the reported errors when opening a plugin details lightbox in WP 7.0.
In my vanilla test site on /wp-admin/plugin-install.php, the load-scripts.php script src looks like:
/wp-admin/load-scripts.php?c=0&load%5Bchunk_0%5D=jquery-core,jquery-migrate,utils&ver=7.0
So it begins with jQuery and the source is:
/*! jQuery v3.7.1 | (c) OpenJS Foundation and other contributors | jquery.org/license */
!function(e,t){"use strict";
...
So the file does not start with "use strict". So the reproduction steps don't seem to be valid.
#15
@
2 weeks ago
- Keywords needs-testing added
@bvedgie Since I cannot reproduce the original issue, please test that the changes address the issue for you.
#16
@
2 weeks ago
I tested PR #12371 with a plugin that looks like this:
<?php /* Plugin Name: Enqueue Hooks Script Early */ if ( is_admin() ) { wp_enqueue_script( 'example-script', plugin_dir_url( __FILE__ ) . 'example-script.js', array( 'wp-hooks', 'thickbox' ), '1', false ); }
The file example-script.js can simply be empty - its only purpose is to load the dependencies.
(As an aside, the above code does not really look right because it is not using the admin_enqueue_scripts hook. But that was the only way I could figure out to reproduce the issue.)
Before applying PR #12371: I got the imgLoader is not defined error.
After applying PR #12371: the thickbox popup seems to be working without any errors.
#17
@
2 weeks ago
- Milestone 7.0.1 → 7.0.2
As the release candidate for 7.0.1 is starting now we'll have to punt this ticket to 7.0.2
#18
follow-up:
↓ 19
@
2 weeks ago
It seems that the PR #12371 will fix the issue with Thickbox, but the issue is likely much broader. The majority of scripts being enqueued in core are in sloppy mode, so simply fixing the issue with Thickbox will just fix the symptom of a much deeper problem.
I had Claude Opus 4.8 categorize the core scripts based on the strict mode:
| Bucket | Count | What it is |
|---|---|---|
File-level 'use strict' (poisoners) | 53 | All 53 are @wordpress/* webpack dist packages (+ react-jsx-runtime) — wp-hooks, wp-i18n, wp-dom-ready, wp-a11y, wp-url, wp-data, wp-element, … Zero are non-/dist/.
|
Function-form 'use strict' (safe) | 91 | Strict, but scoped inside IIFEs/functions — can't leak. |
| Sloppy (potential victims) | 114 | 98 core-authored (common, post, editor, heartbeat, media-views, updates, wp-lists, customize-controls, nav-menu, plugin-install, postbox, wplink, …) + 15 vendor.
|
This was run with the PR active, so thickbox is included among the function-form list of 91. Otherwise, in trunk there were 115 sloppy-mode scripts detected.
As @siliconforks commented, 53 core-bundled scripts which use file-level use strict all come from Gutenberg's build process. So if we modified that to wrap the output in an IIFE that had the function form of 'use strict', then that would also definitely solve the problem. Since only core-bundled scripts are eligible for concatenation, there wouldn't be an opportunity for a plugin or theme to cause the problem again with their own file-level use strict. However, we can't just simply wrap all all of these as-is. The generated wp-includes/js/dist/hooks.js starts with:
"use strict"; var wp; (wp ||= {}).hooks = (() => {
Wrapping in an IIFE would prevent the wp global from being defined. Bad.
The esbuild process seems it should be updated to do this differently:
(function() { "use strict"; window.wp ||= {}; window.wp.hooks = (() => {
Nevertheless, the issue could crop up again if we bundle a file-level use strict to core. So I think the better solution for this and other headaches is to stop concatenating scripts (#57548).
#19
in reply to: ↑ 18
@
2 weeks ago
Replying to westonruter:
The esbuild process seems it should be updated to do this differently:
(function() { "use strict"; window.wp ||= {}; window.wp.hooks = (() => {
I've opened https://github.com/WordPress/gutenberg/pull/79792 to implement this.
This ticket was mentioned in PR #12387 on WordPress/wordpress-develop by @westonruter.
2 weeks ago
#20
This updates core to use Gutenberg with the changes in https://github.com/WordPress/gutenberg/pull/79792 for testing.
Trac ticket: https://core.trac.wordpress.org/ticket/65515
## Use of AI Tools
None for this PR, but see the Gutenberg PR for its AI disclosure.
#21
@
2 weeks ago
I've also opened PR #12387 just for testing core with the changes esbuild changes.
The diff in src/wp-includes/js/dist/hooks.js shows how 'use strict' has moved from file-level to function-level:
-
src/wp-includes/js/dist/hooks.js
1 (function() { 1 2 "use strict"; 2 3 var wp; 3 4 (wp ||= {}).hooks = (() => { … … 373 374 } = defaultHooks; 374 375 return __toCommonJS(index_exports); 375 376 })(); 377 (window.wp ||= {}).hooks = wp.hooks; 378 })(); 376 379 //# sourceMappingURL=index.js.map
@westonruter commented on PR #12387:
2 weeks ago
#22
@desrosj FYI: I updated gutenberg.sha to point to a PR and it causes unit test failures unexpectedly.
@desrosj commented on PR #12387:
10 days ago
#23
@westonruter it looks like these are legitimate failures occurring based on the differences in the GB-sourced code at first glance. Unfortunately, PHPUnit test changes that are required need to be applied manually in the PR because there's no way to map tests in the Gutenberg repository to wordpress-develop. Unless I am missing something.
@westonruter commented on PR #12387:
10 days ago
#24
@desrosj ah, ok. In this case, since this PR is only for testing purposes (to get the Gutenberg JS packages applied to core), then probably not worth addressing those unit test errors. This PR wouldn't be committed to to SVN anyway.
#25
@
4 hours ago
Tested on: WordPress 7.0
Before:
With script concatenation enabled, clicking "View details" on the Plugins page displayed the ThickBox overlay, but the modal failed to load.
The browser console showed the following JavaScript error:
ReferenceError: imgLoader is not defined
After:
Setting CONCATENATE_SCRIPTS to false allowed the plugin details modal to open correctly.
The JavaScript error no longer appeared.
Results:
I was able to reproduce the reported issue.
The behavior matches the ticket description.
Screenshots have been attached for reference.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
See also #57548 which proposes eliminating the concatenation altogether.
Is this truly a new issue in 7.0? If so, what commit was it introduced in?