Skip to content

fix(toc): expose tableOfContents on globalThis#188

Merged
JohnMcLear merged 2 commits intomasterfrom
fix/global-scope-toc
Apr 23, 2026
Merged

fix(toc): expose tableOfContents on globalThis#188
JohnMcLear merged 2 commits intomasterfrom
fix/global-scope-toc

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Problem

postAceInit.js fails on pad load in Chromium with:

Uncaught (in promise) ReferenceError: tableOfContents is not defined

The error blocks subsequent pad initialization. Firefox swallows it as an unhandled promise rejection, so the bug went undetected there — which is why it shipped without CI catching it.

Root cause

static/js/toc.js declares const tableOfContents = {...} at the top level of a regular <script> tag. Top-level const is script-scoped, not a global — it's not attached to window.

postAceInit.js and aceEditEvent.js are loaded by Etherpad's plugin framework as CommonJS-style modules in their own scopes, so their bare tableOfContents identifier resolves via the scope chain up to globalThis — which never had the object attached.

Fix

One-line change: assign to globalThis.tableOfContents alongside the const declaration. globalThis resolves to window in the browser and to global in Node.js, so the unit tests under tests/ (which evaluate toc.js inside a Node vm.runInNewContext) keep working — using window instead would have broken them.

Test plan

  • npm test passes locally (6/6)
  • npm run lint — no new errors in static/js/toc.js
  • Verified in live Chromium by loading a pad with this plugin enabled — no `ReferenceError`, TOC button works.

🤖 Generated with Claude Code

@JohnMcLear
Copy link
Copy Markdown
Member Author

/review

…each it

Top-level `const` in a <script> tag is script-scoped, not a global.
`postAceInit.js` and `aceEditEvent.js` are loaded by Etherpad's plugin
framework as CommonJS-style modules in their own scopes, so their bare
`tableOfContents` identifier resolves via the scope chain up to
globalThis — which never had the object attached.

On Firefox the resulting `Uncaught (in promise) ReferenceError:
tableOfContents is not defined` is swallowed silently as an unhandled
promise rejection, so the pad appears to work. On Chromium it surfaces
loudly and blocks pad initialization.

Fix: assign to `globalThis.tableOfContents` alongside the `const`
declaration. `globalThis` resolves to `window` in the browser and to
`global` in Node.js, so the unit tests under `tests/` (which evaluate
toc.js inside a Node vm.runInNewContext) keep working — `window` would
have broken them.
…copes

The toc.js fix alone was not enough. `aceEditEvent` fires inside ACE's
inner iframe and `postAceInit` is loaded as a CommonJS plugin hook
module with its own scope — in both cases `globalThis` is not shared
with the <script> tag that loads toc.js, so the bare `tableOfContents`
identifier still threw ReferenceError on Chromium.

Both hook modules now look up the object via a small `getToc()` helper
that tries globalThis, then window.top, then window — so it works
regardless of which realm the hook happens to run in. If the lookup
fails (e.g. toc.js has not executed yet), the hook silently no-ops and
the next invocation picks it up.

Verified live in Chromium after this change: no ReferenceError in the
console, pad initializes cleanly, TOC toggle works. Firefox continues
to work (it was only silent about the bug, not immune to it).
@JohnMcLear JohnMcLear force-pushed the fix/global-scope-toc branch from f4d651d to 950cd5f Compare April 23, 2026 08:39
@JohnMcLear JohnMcLear merged commit 8d9eef7 into master Apr 23, 2026
3 checks passed
@JohnMcLear JohnMcLear deleted the fix/global-scope-toc branch April 23, 2026 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant