Merge pull request #6019 from anthonykim1/anthonykim1/fix-terminal-window-mouse-listeners

Use current document for mouse listeners

Author: anthonykim1

src/browser/services/MouseService.test.ts

@@ -351,3 +351,96 @@ describe('MouseService mouseEventsRequireAlt', () => {
     assert.isFalse(element.classList.contains(MouseEventCssClasses.ENABLE_MOUSE_EVENTS));
   });
 });
+
+describe('MouseService multi-window', () => {
+  interface IRecordingDocument {
+    doc: Document;
+    added: string[];
+    removed: string[];
+    listeners: { [type: string]: EventListener[] };
+  }
+
+  function createRecordingDocument(): IRecordingDocument {
+    const added: string[] = [];
+    const removed: string[] = [];
+    const listeners: { [type: string]: EventListener[] } = {};
+    const doc = {
+      addEventListener: (type: string, listener: EventListener) => {
+        added.push(type);
+        (listeners[type] ??= []).push(listener);
+      },
+      removeEventListener: (type: string) => removed.push(type)
+    } as any;
+    return { doc, added, removed, listeners };
+  }
+
+  it('should manage protocol mouseup/mousemove listeners on the element\'s current document, not the document the terminal was opened with', () => {
+    const mouseStateService = new MouseStateService();
+    const optionsService = new OptionsService({});
+    const mouseService = new MouseService(
+      new MockRenderService(),
+      {
+        getMouseReportCoords: () => ({ col: 0, row: 0, x: 0, y: 0 })
+      } as any,
+      mouseStateService,
+      {
+        triggerDataEvent: () => {},
+        triggerBinaryEvent: () => {},
+        decPrivateModes: { applicationCursorKeys: false }
+      } as any,
+      bufferService,
+      optionsService,
+      new TestSelectionService(),
+      logService,
+      new MockCoreBrowserService()
+    );
+
+    // Simulate a terminal that was opened with one document but whose element now lives in
+    // another document (eg. it was moved to a child window)
+    const openDocument = createRecordingDocument();
+    const childDocument = createRecordingDocument();
+    const elementListeners: { [type: string]: EventListener[] } = {};
+    const element = {
+      classList: {
+        add: () => {},
+        remove: () => {},
+        contains: () => false
+      },
+      addEventListener: (type: string, listener: EventListener) => {
+        (elementListeners[type] ??= []).push(listener);
+      },
+      removeEventListener: () => {},
+      ownerDocument: childDocument.doc
+    } as any;
+
+    mouseService.bindMouse({
+      element,
+      screenElement: createTestMouseTargetElement(),
+      document: openDocument.doc
+    }, disposable => disposable, () => {});
+    mouseStateService.activeProtocol = 'ANY';
+
+    elementListeners['mousedown'][0]({
+      type: 'mousedown',
+      button: 0,
+      buttons: 1,
+      altKey: false,
+      ctrlKey: false,
+      shiftKey: false,
+      preventDefault: () => {}
+    } as any);
+    assert.deepEqual(childDocument.added, ['mouseup', 'mousemove']);
+    assert.deepEqual(openDocument.added, []);
+
+    childDocument.listeners['mouseup'][0]({
+      type: 'mouseup',
+      button: 0,
+      buttons: 0,
+      altKey: false,
+      ctrlKey: false,
+      shiftKey: false
+    } as any);
+    assert.deepEqual(childDocument.removed, ['mouseup', 'mousemove']);
+    assert.deepEqual(openDocument.removed, []);
+  });
+});

src/browser/services/MouseService.ts

@@ -7,7 +7,7 @@ import { addDisposableListener } from '../Dom';
 import { IBufferService, IMouseStateService, ICoreService, ILogService, IOptionsService } from '../../common/services/Services';
 import { CoreMouseAction, CoreMouseButton, CoreMouseEventType, ICoreMouseEvent, IDisposable } from '../../common/Types';
 import { C0 } from '../../common/data/EscapeSequences';
-import { DisposableStore, MutableDisposable, toDisposable } from '../../common/Lifecycle';
+import { DisposableStore, MutableDisposable } from '../../common/Lifecycle';
 import { ICoreBrowserService, IMouseCoordsService, IMouseService, IMouseServiceTarget, IRenderService, ISelectionService } from './Services';
 import { Gesture, EventType as GestureEventType, IGestureEvent } from '../scrollable/touch';
 
@@ -21,6 +21,8 @@ interface IMouseBindContext {
   readonly target: IMouseServiceTarget;
   readonly focus: () => void;
   readonly requestedEvents: RequestedMouseEvents;
+  readonly mouseupListener: MutableDisposable<IDisposable>;
+  readonly mousedragListener: MutableDisposable<IDisposable>;
 }
 
 export class MouseService implements IMouseService {
@@ -61,7 +63,11 @@ export class MouseService implements IMouseService {
       mousedrag: null,
       mousemove: null
     };
-    const ctx: IMouseBindContext = { target, focus, requestedEvents };
+    const mouseupListener = new MutableDisposable<IDisposable>();
+    const mousedragListener = new MutableDisposable<IDisposable>();
+    register(mouseupListener);
+    register(mousedragListener);
+    const ctx: IMouseBindContext = { target, focus, requestedEvents, mouseupListener, mousedragListener };
     const eventListeners: Record<'mouseup' | 'wheel' | 'mousedrag' | 'mousemove', EventListener> = {
       mouseup: (ev: Event) => this._handleMouseUp(ctx, ev as MouseEvent),
       wheel: (ev: Event) => this._handleWheel(ctx, ev as WheelEvent),
@@ -85,16 +91,6 @@ export class MouseService implements IMouseService {
     // force initial onProtocolChange so we dont miss early mouse requests
     this._mouseStateService.activeProtocol = this._mouseStateService.activeProtocol;
 
-    // Ensure document-level listeners are removed on dispose
-    register(toDisposable(() => {
-      if (requestedEvents.mouseup) {
-        document.removeEventListener('mouseup', requestedEvents.mouseup);
-      }
-      if (requestedEvents.mousedrag) {
-        document.removeEventListener('mousemove', requestedEvents.mousedrag);
-      }
-    }));
-
     /**
      * "Always on" event listeners.
      */
@@ -199,12 +195,8 @@ export class MouseService implements IMouseService {
     this._sendEvent(ctx, ev);
     if (!ev.buttons) {
       // if no other button is held remove global handlers
-      if (ctx.requestedEvents.mouseup) {
-        ctx.target.document.removeEventListener('mouseup', ctx.requestedEvents.mouseup);
-      }
-      if (ctx.requestedEvents.mousedrag) {
-        ctx.target.document.removeEventListener('mousemove', ctx.requestedEvents.mousedrag);
-      }
+      ctx.mouseupListener.clear();
+      ctx.mousedragListener.clear();
     }
   }
 
@@ -246,11 +238,14 @@ export class MouseService implements IMouseService {
     // of the terminal element.
     // Note: Other emulators also do this for 'mousedown' while a button
     // is held, we currently limit 'mousedown' to the terminal only.
+    // Use the element's current document in case it moved to another window after open.
+    const { element, document: targetDocument } = ctx.target;
+    const listenerDocument = element.ownerDocument ?? targetDocument;
     if (ctx.requestedEvents.mouseup) {
-      ctx.target.document.addEventListener('mouseup', ctx.requestedEvents.mouseup);
+      ctx.mouseupListener.value = addDisposableListener(listenerDocument, 'mouseup', ctx.requestedEvents.mouseup);
     }
     if (ctx.requestedEvents.mousedrag) {
-      ctx.target.document.addEventListener('mousemove', ctx.requestedEvents.mousedrag);
+      ctx.mousedragListener.value = addDisposableListener(listenerDocument, 'mousemove', ctx.requestedEvents.mousedrag);
     }
   }
 
@@ -398,7 +393,7 @@ export class MouseService implements IMouseService {
   }
 
   private _handleProtocolChange(ctx: IMouseBindContext, eventListeners: Record<'mouseup' | 'wheel' | 'mousedrag' | 'mousemove', EventListener>, events: CoreMouseEventType): void {
-    const { element, document } = ctx.target;
+    const { element } = ctx.target;
     const { requestedEvents } = ctx;
     // apply global changes on events
     if (events) {
@@ -433,18 +428,14 @@ export class MouseService implements IMouseService {
     }
 
     if (!(events & CoreMouseEventType.UP)) {
-      if (requestedEvents.mouseup) {
-        document.removeEventListener('mouseup', requestedEvents.mouseup);
-      }
+      ctx.mouseupListener.clear();
       requestedEvents.mouseup = null;
     } else {
       requestedEvents.mouseup ??= eventListeners.mouseup;
     }
 
     if (!(events & CoreMouseEventType.DRAG)) {
-      if (requestedEvents.mousedrag) {
-        document.removeEventListener('mousemove', requestedEvents.mousedrag);
-      }
+      ctx.mousedragListener.clear();
       requestedEvents.mousedrag = null;
     } else {
       requestedEvents.mousedrag ??= eventListeners.mousedrag;