Add explicit variables to fix use strict.

[aaronjorbin]

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.

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props jorbin, westonruter, siliconforks.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[Copilot]

Pull request overview

This PR updates the vendored Thickbox script to be safe when executed under strict mode (e.g., when concatenated after a "use strict" script) by making previously implicit globals explicit, and adds a QUnit regression test to ensure Thickbox initializes under strict mode.

Changes:

• Declare previously implicit Thickbox state variables (imgLoader, TB_*) explicitly to avoid strict-mode ReferenceErrors.
• Convert a few other implicit/global assignments inside tb_show() / tb_getPageSize() into properly scoped locals / direct returns.
• Add a QUnit test that loads the Thickbox source and evaluates it under strict mode, asserting initialization does not throw.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/qunit/wp-includes/js/thickbox.js	Adds a strict-mode initialization regression test for Thickbox.
src/js/_enqueues/vendor/thickbox/thickbox.js	Makes Thickbox variables explicitly declared/scoped to avoid strict-mode failures.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[westonruter]

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]

Adding this seems safer than just relying on loading it with QUnit as strict mode, because some strict mode semantics would only affect the behavior at runtime. And since QUnit isn't attempting to do full test coverage, there would be issues which possibly aren't detected. However, since now JSHint is checking the file for issues (b4ed2e7), and the only strict-mode specific error is W040 ("JSHint: If a strict mode function is executed using function invocation, its 'this' value will be undefined.") which I've confirmed to not be an issue in tb_click() since it is passed to jQuery as an event handler, I think it's better to include use strict so that any strict mode issues can be detected with static analysis.

[siliconforks]

Looking at the big picture - isn't adding use strict at the top of the file just making the problem worse?

Now if thickbox.js is concatenated with other scripts, and thickbox.js happens to be the first script, it will put all scripts in the concatenation into strict mode. If some of the other scripts expect to be run in sloppy mode, that could cause issues with those scripts.

(As a compromise, you could put use strict into each of the individual outermost functions instead. There are 10 functions, so you would need to repeat it 10 times. But it should be safer than putting it at the top of the file.)

[westonruter]

That's a good point. I suppose it depends a bit on how many core scripts are currently in strict of sloppy mode. If thickbox is the outlier in being sloppy, then the way it is here seems fine. Otherwise, if there are many cases of sloppy mode scripts then your suggestion is probably the better way to go.

[westonruter]

I've switched to using the function form of use strict in aedfc3e

An added benefit is it eliminates the need to suppress the W097 warning (Use the function form of 'use strict') in JSHint.

[westonruter]

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

• imgPreloader has 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 in jquery.prettyPhoto.js, which declares and uses its own imgPreloader. None of these read WordPress core's value.
• ajaxContentW/H, urlNoQuery, arrayPageSize are 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 via if ( 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-level var (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_pathToImage override 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.

[westonruter]

I tried opening the plugin details lightbox in Playground with this PR, and it works. However, it works for me in 7.0 as well.

[westonruter]

While making Thickbox strict-mode compatible is attractive, I think the better approach is WordPress/gutenberg#79792 since it is comprehensive, specifically targeting the original regression that caused the issue in the first place. It also avoids the need to patch vendor libraries like this (although surely such patching is not unique).