Skip to content

Fix dialog title bars and env-switch crash from the Electron 42 / TS6 migration#1022

Merged
krassowski merged 2 commits into
jupyterlab:masterfrom
notluquis:fix/electron42-dialog-window-regressions
Jul 3, 2026
Merged

Fix dialog title bars and env-switch crash from the Electron 42 / TS6 migration#1022
krassowski merged 2 commits into
jupyterlab:masterfrom
notluquis:fix/electron42-dialog-window-regressions

Conversation

@notluquis

Copy link
Copy Markdown
Collaborator

Fixes both dialog and session window regressions from #1021. Both come from the Electron 42 (#988) and TypeScript 6 (#978) migrations, not from anything in JupyterLab.

Dialog title bars (all ThemedWindow dialogs)

dialogtitlebar.ts is injected into a data: URL page as an inline module, but tsconfig module: nodenext emits it as CommonJS, so exports is not defined throws before customElements.define runs and the element never registers. The first commit defines the CommonJS globals ahead of the injected script so it registers again. This is the surgical fix. Moving these dialogs off the data: URL to loadFile or a privileged scheme, which also lets a real CSP apply, is the proper follow up and I have it noted for a later PR.

Env switch crash and listener leak

The window and titlebar focus handlers closed over a stale labView; after an env switch its webContents is undefined under WebContentsView and the handler threw Cannot read properties of undefined (reading 'focus'). The second commit registers the two handlers once and reads the current this._labView, which also stops them accumulating on the persistent emitters, and guards the post-resize invalidate() against a closed webContents.

Testing

Added test/e2e/dialog-titlebar.test.ts, which opens a dialog and asserts the title bar custom element registers and renders its close button. I confirmed it fails on the pre-fix build and passes after. The full e2e suite is green (11 of 11) including the session window tests, and type-check and build are clean.

Closes #1021

I put this together and traced it with Claude Code, and verified everything against the running app and the local e2e suite before opening this.

notluquis added 2 commits July 3, 2026 16:20
themedwindow injects the compiled dialogtitlebar into a data: URL page as an
inline ES module, but tsconfig module: nodenext emits that file as CommonJS, so
it references exports, throws "exports is not defined", and aborts before
customElements.define runs. Every ThemedWindow dialog then loses its title bar
and close button. Define the CommonJS globals ahead of the injected script so the
registration side effect runs. Moving these dialogs off the data: URL is the real
fix and is tracked for later.

Add an e2e test that opens a dialog and asserts the title bar element registers,
so this regression class is caught next time.

Built with Claude Code; verified against the running app and the full e2e suite
locally, and confirmed the new test fails on the pre-fix build.
…date

The window and titlebar 'focus' handlers dereferenced a labView captured in the
closure. On an env switch the old labView is disposed and, under WebContentsView,
its webContents goes undefined, so the stale handler threw "Cannot read
properties of undefined (reading 'focus')". Register the two handlers once and
dereference the current this._labView instead, which also stops them accumulating
on the persistent window and titlebar emitters on every switch. Guard the
post-resize invalidate() against an already-closed webContents in the resize then
dispose race.

Built with Claude Code; verified by switching environments in the running app and
against the full e2e suite locally.
Copilot AI review requested due to automatic review settings July 3, 2026 20:23
@notluquis notluquis added the bug label Jul 3, 2026
@notluquis notluquis self-assigned this Jul 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@notluquis notluquis requested a review from krassowski July 3, 2026 20:24
@krassowski krassowski merged commit e050646 into jupyterlab:master Jul 3, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dialog title bars don't render and switching Python environment can crash (Electron 42 / TS6 migration)

3 participants