Deregister Arabic shaping joiner when disposing pane

Since xterm.js does not automatically remove registered character joiners
upon terminal disposal, they must be explicitly deregistered to avoid
memory and registration leaks.

- Update `registerArabicShapingJoiner` to return a cleanup function.
- Store and call the cleanup handler when disposing a managed pane.
- Update tests to assert proper registration and deregistration.
This commit is contained in:
Jinjing
2026-07-08 17:05:32 -07:00
parent d1a119b4c8
commit dc2c74c89b
5 changed files with 33 additions and 8 deletions
@@ -148,6 +148,7 @@ export function createPaneDOM(
compositionHandler: null,
focusClassSyncCleanup: null,
terminalScrollIntentDisposable: null,
arabicShapingJoinerCleanup: null,
pendingSplitScrollState: null,
pendingSplitScrollRafIds: [],
pendingSplitScrollTimerId: null,
@@ -76,7 +76,7 @@ export function openTerminal(pane: ManagedPaneInternal): void {
// Why: without run-joining, Arabic/Hebrew output renders as disconnected
// letters in reversed order (#5262). Registered up front so restored
// scrollback and reattach replays shape correctly, not just live output.
registerArabicShapingJoiner(terminal)
pane.arabicShapingJoinerCleanup = registerArabicShapingJoiner(terminal)
// Why: the OS reads the focused textarea's screen rect at compositionstart to
// decide where to display the IME candidate window. xterm positions that
@@ -230,6 +230,13 @@ export function disposePane(
pane.focusClassSyncCleanup = null
pane.terminalScrollIntentDisposable?.dispose()
pane.terminalScrollIntentDisposable = null
// Deregister the RTL shaping joiner: terminal.dispose() below does not.
try {
pane.arabicShapingJoinerCleanup?.()
} catch {
/* ignore */
}
pane.arabicShapingJoinerCleanup = null
if (pane.compositionHandler) {
pane.terminal.element?.removeEventListener('compositionstart', pane.compositionHandler)
pane.terminal.element?.removeEventListener('compositionupdate', pane.compositionHandler)
@@ -161,6 +161,9 @@ export type ManagedPaneInternal = {
focusClassSyncCleanup?: (() => void) | null
// Stored so disposePane() can remove user-scroll intent listeners.
terminalScrollIntentDisposable?: IDisposable | null
// Stored so disposePane() can deregister the joiner; terminal.dispose()
// does not remove registered character joiners.
arabicShapingJoinerCleanup?: (() => void) | null
// Why: splitPane reparents DOM; its delayed restore owns scroll until the
// browser settles, so intermediate fits must not compete with it.
pendingSplitScrollState: ScrollState | null
@@ -122,15 +122,24 @@ describe('findRtlJoinRanges', () => {
})
describe('registerArabicShapingJoiner', () => {
it('registers the range finder with the terminal and returns the joiner id', () => {
it('registers the range finder and returns a cleanup that deregisters it', () => {
let registered: ((text: string) => [number, number][]) | null = null
let deregistered: number | null = null
const terminal = {
registerCharacterJoiner(handler: (text: string) => [number, number][]): number {
registered = handler
return 7
},
deregisterCharacterJoiner(joinerId: number): void {
deregistered = joinerId
}
}
expect(registerArabicShapingJoiner(terminal)).toBe(7)
const cleanup = registerArabicShapingJoiner(terminal)
expect(registered).toBe(findRtlJoinRanges)
expect(deregistered).toBeNull()
// terminal.dispose() does not deregister joiners, so cleanup must.
cleanup()
expect(deregistered).toBe(7)
})
})
@@ -116,10 +116,15 @@ export function findRtlJoinRanges(text: string): [number, number][] {
return ranges
}
/** Register the RTL shaping joiner on a terminal. Registration is per
* terminal instance and is torn down with the terminal itself. */
/** Register the RTL shaping joiner on a terminal. Returns a cleanup that
* deregisters the joiner — `Terminal.dispose()` does not remove registered
* character joiners, so disposePane() must call this to avoid leaking the
* registration (xtermjs/xterm.js#3289). */
export function registerArabicShapingJoiner(
terminal: Pick<Terminal, 'registerCharacterJoiner'>
): number {
return terminal.registerCharacterJoiner(findRtlJoinRanges)
terminal: Pick<Terminal, 'registerCharacterJoiner' | 'deregisterCharacterJoiner'>
): () => void {
const joinerId = terminal.registerCharacterJoiner(findRtlJoinRanges)
return () => {
terminal.deregisterCharacterJoiner(joinerId)
}
}