You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In 6.0.0, the document-level mouseup/mousemove listeners that back mouse reporting are attached directly to the document on mousedown and removed only from inside the mouseup handler. They are not tied to the terminal's disposable chain, so a Terminal.dispose() that happens while a button is held leaves them attached. The next mouseup anywhere on the page then runs against the disposed terminal and throws:
Uncaught TypeError: Cannot read properties of undefined (reading 'dimensions')
at get dimensions (@xterm_xterm.js:4641:33)
at Xt.getMouseReportCoords (@xterm_xterm.js:4493:107)
at r (@xterm_xterm.js:8643:31)
at HTMLDocument.mouseup (@xterm_xterm.js:8667:102)
The mousemove (mousedrag) variant throws the same way.
It looks like this is already fixed on master by #6019 ("use disposables for transient mouse listeners", 1701dce) — the listeners are now MutableDisposables registered through bindMouse's register callback, so they come down with the terminal. Filing anyway because 6.0.0 is what's on npm and the fix is unreleased, and because there's a second half that master still has (below).
Version
@xterm/xterm 6.0.0 (current npm latest)
Electron 38 renderer, also reproducible in Chrome
Precondition: an application that has enabled mouse reporting (DECSET 1000/1002/1003) — e.g. tmux, vim, a TUI. Without mouse reporting no document listener is requested and nothing throws.
Mechanism (at tag 6.0.0)
src/browser/CoreBrowserTerminal.ts, in the mousedown handler:
and the only removal path for those two, inside the mouseup listener itself:
mouseup: (ev: MouseEvent)=>{sendEvent(ev);if(!ev.buttons){// if no other button is held remove global handlersthis._document!.removeEventListener('mouseup',requestedEvents.mouseup!);if(requestedEvents.mousedrag){this._document!.removeEventListener('mousemove',requestedEvents.mousedrag);}}returnthis.cancel(ev);},
So between mousedown and mouseup there is a window where the listeners are live but owned by nothing. Disposing in that window leaves them on the document forever, closed over a dead terminal.
Two things make it worse than a one-off:
sendEvent(ev) runs before the removeEventListener calls. Since sendEvent is what throws, the removal never executes — so the stale listeners stay attached and every subsequent mouseup on the page throws again. In our app it went from one remount to dozens of uncaught errors in a session.
The listeners are on document, so the throws continue even when the pointer is nowhere near a terminal.
_renderer is a MutableDisposable, cleared on dispose, so .value is undefined and the ! turns it into the uncaught TypeError.
Repro
constterm=newTerminal();term.open(document.getElementById('terminal'));// application enables mouse reporting (what tmux/vim do)term.write('\x1b[?1002h');// wait a frame for the renderer to come up, then:constscreen=document.querySelector('.xterm-screen');screen.dispatchEvent(newMouseEvent('mousedown',{bubbles: true,button: 0,buttons: 1,clientX: 50,clientY: 50}));term.dispose();// the release the terminal never got to seedocument.dispatchEvent(newMouseEvent('mouseup',{bubbles: true,button: 0,buttons: 0,clientX: 50,clientY: 50}));// -> Uncaught TypeError: Cannot read properties of undefined (reading 'dimensions')
By hand: enable mouse reporting, press and hold the left button over the terminal, dispose the terminal while the button is down (in a React app, unmounting the component is enough), release the button.
This is not exotic in an embedder — any remount while the user is mid-drag hits it. Ours came from workspace switches and reconnects that tear down and rebuild terminals; the drag spans the remount.
Ask
Can you confirm Use current document for mouse listeners #6019 is the fix for the listener half, and whether it's expected to land in a 6.0.x patch or only in the next minor? Embedders on 6.0.0 currently have no clean way to avoid this other than deferring their own dispose() until the button is released, which is what we ended up doing.
RenderService.dimensions is still this._renderer.value!.dimensions on master — the only unguarded _renderer.value access in that file; everything else uses ?. or an if (!this._renderer.value) early return. With Use current document for mouse listeners #6019 in place I don't have a trigger for it anymore, but it's the one spot that converts "something held a reference across dispose" into an uncaught TypeError from inside a getter rather than a no-op. Happy to send a small PR that guards it (and a unit test for dispose-then-mouseup) if you think that's worth having — say the word and I'll open one against master.
Summary
In 6.0.0, the document-level
mouseup/mousemovelisteners that back mouse reporting are attached directly to the document onmousedownand removed only from inside themouseuphandler. They are not tied to the terminal's disposable chain, so aTerminal.dispose()that happens while a button is held leaves them attached. The nextmouseupanywhere on the page then runs against the disposed terminal and throws:The
mousemove(mousedrag) variant throws the same way.It looks like this is already fixed on master by #6019 ("use disposables for transient mouse listeners", 1701dce) — the listeners are now
MutableDisposables registered throughbindMouse'sregistercallback, so they come down with the terminal. Filing anyway because 6.0.0 is what's on npm and the fix is unreleased, and because there's a second half that master still has (below).Version
@xterm/xterm6.0.0 (current npm latest)Mechanism (at tag 6.0.0)
src/browser/CoreBrowserTerminal.ts, in themousedownhandler:and the only removal path for those two, inside the
mouseuplistener itself:So between
mousedownandmouseupthere is a window where the listeners are live but owned by nothing. Disposing in that window leaves them on the document forever, closed over a dead terminal.Two things make it worse than a one-off:
sendEvent(ev)runs before theremoveEventListenercalls. SincesendEventis what throws, the removal never executes — so the stale listeners stay attached and every subsequentmouseupon the page throws again. In our app it went from one remount to dozens of uncaught errors in a session.document, so the throws continue even when the pointer is nowhere near a terminal.The throw itself lands in
RenderService:_rendereris aMutableDisposable, cleared on dispose, so.valueisundefinedand the!turns it into the uncaught TypeError.Repro
By hand: enable mouse reporting, press and hold the left button over the terminal, dispose the terminal while the button is down (in a React app, unmounting the component is enough), release the button.
This is not exotic in an embedder — any remount while the user is mid-drag hits it. Ours came from workspace switches and reconnects that tear down and rebuild terminals; the drag spans the remount.
Ask
Can you confirm Use current document for mouse listeners #6019 is the fix for the listener half, and whether it's expected to land in a 6.0.x patch or only in the next minor? Embedders on 6.0.0 currently have no clean way to avoid this other than deferring their own
dispose()until the button is released, which is what we ended up doing.RenderService.dimensionsis stillthis._renderer.value!.dimensionson master — the only unguarded_renderer.valueaccess in that file; everything else uses?.or anif (!this._renderer.value)early return. With Use current document for mouse listeners #6019 in place I don't have a trigger for it anymore, but it's the one spot that converts "something held a reference across dispose" into an uncaught TypeError from inside a getter rather than a no-op. Happy to send a small PR that guards it (and a unit test for dispose-then-mouseup) if you think that's worth having — say the word and I'll open one against master.