Improve dispoable listener functions

src/AccessibilityManager.ts

@@ -6,6 +6,7 @@
 import { ITerminal, IBuffer, IDisposable } from './Interfaces';
 import { isMac } from './utils/Browser';
 import { RenderDebouncer } from './utils/RenderDebouncer';
+import { addDisposableListener } from './utils/Dom';
 
 const MAX_ROWS_TO_READ = 20;
 const ACTIVE_ITEM_ID_PREFIX = 'xterm-active-item-';
@@ -55,28 +56,22 @@ export class AccessibilityManager implements IDisposable {
 
     this._terminal.element.appendChild(this._accessibilityTreeRoot);
 
-    this._addTerminalEventListener('resize', data => this._onResize(data.cols, data.rows));
-    this._addTerminalEventListener('refresh', data => this._refreshRows(data.start, data.end));
-    this._addTerminalEventListener('scroll', data => this._refreshRows());
+    this._terminal.addDisposableListener('resize', data => this._onResize(data.cols, data.rows));
+    this._terminal.addDisposableListener('refresh', data => this._refreshRows(data.start, data.end));
+    this._terminal.addDisposableListener('scroll', data => this._refreshRows());
     // Line feed is an issue as the prompt won't be read out after a command is run
-    this._addTerminalEventListener('a11y.char', (char) => this._onChar(char));
-    this._addTerminalEventListener('linefeed', () => this._onChar('\n'));
-    // Ensure \t is covered, if not 2 words separated by only a tab will be read as 1 word
-    this._addTerminalEventListener('a11y.tab', spaceCount => {
-      for (let i = 0; i < spaceCount; i++) {
-        this._onChar(' ');
-      }
-    });
-    this._addTerminalEventListener('charsizechanged', () => this._refreshRowsDimensions());
-    this._addTerminalEventListener('key', keyChar => this._onKey(keyChar));
-    this._addTerminalEventListener('blur', () => this._clearLiveRegion());
+    this._terminal.addDisposableListener('a11y.char', (char) => this._onChar(char));
+    this._terminal.addDisposableListener('linefeed', () => this._onChar('\n'));
+    this._terminal.addDisposableListener('a11y.tab', spaceCount => this._onTab(spaceCount));
+    this._terminal.addDisposableListener('charsizechanged', () => this._refreshRowsDimensions());
+    this._terminal.addDisposableListener('key', keyChar => this._onKey(keyChar));
+    this._terminal.addDisposableListener('blur', () => this._clearLiveRegion());
     // TODO: Maybe renderer should fire an event on terminal when the characters change and that
     //       should be listened to instead? That would mean that the order of events are always
     //       guarenteed
-    this._addTerminalEventListener('dprchange', () => this._refreshRowsDimensions());
+    this._terminal.addDisposableListener('dprchange', () => this._refreshRowsDimensions());
     // TODO: Dispose of this listener when disposed
-    // TODO: Listen instead to when devicePixelRatio changed (depends on PR #1172)
-    window.addEventListener('resize', () => this._refreshRowsDimensions());
+    addDisposableListener(window, 'resize', () => this._refreshRowsDimensions());
 
     this._rowContainer.addEventListener('keyup', e => {
       if (this._navigationMode.isActive) {
@@ -92,15 +87,6 @@ export class AccessibilityManager implements IDisposable {
     });
   }
 
-  private _addTerminalEventListener(type: string, listener: (...args: any[]) => any): void {
-    this._terminal.on(type, listener);
-    this._disposables.push({
-      dispose: () => {
-        this._terminal.off(type, listener);
-      }
-    });
-  }
-
   public dispose(): void {
     this._renderRowsDebouncer.dispose();
     this._terminal.element.removeChild(this._accessibilityTreeRoot);
@@ -133,6 +119,12 @@ export class AccessibilityManager implements IDisposable {
     return element;
   }
 
+  private _onTab(spaceCount: number): void {
+    for (let i = 0; i < spaceCount; i++) {
+      this._onChar(' ');
+    }
+  }
+
   private _onChar(char: string): void {
     if (this._liveRegionLineCount < MAX_ROWS_TO_READ + 1) {
       // \n needs to be printed as a space, otherwise it will be collapsed to

src/EventEmitter.ts

@@ -3,7 +3,7 @@
  * @license MIT
  */
 
-import { IEventEmitter, IListenerType } from './Interfaces';
+import { IEventEmitter, IListenerType, IDisposable } from './Interfaces';
 
 export class EventEmitter implements IEventEmitter {
   private _events: {[type: string]: IListenerType[]};
@@ -19,6 +19,25 @@ export class EventEmitter implements IEventEmitter {
     this._events[type].push(listener);
   }
 
+  /**
+   * Adds a disposabe listener to the EventEmitter, returning the disposable.
+   * @param type The event type.
+   * @param handler The handler for the listener.
+   */
+  public addDisposableListener(type: string, handler: IListenerType): IDisposable {
+    this.on(type, handler);
+    return {
+      dispose: () => {
+        if (!handler) {
+          // Already disposed
+          return;
+        }
+        this.off(type, handler);
+        handler = null;
+      }
+    };
+  }
+
   public off(type: string, listener: IListenerType): void {
     if (!this._events[type]) {
       return;

src/Interfaces.ts

@@ -244,6 +244,7 @@ export interface IEventEmitter {
   on(type: string, listener: IListenerType): void;
   off(type: string, listener: IListenerType): void;
   emit(type: string, data?: any): void;
+  addDisposableListener(type: string, handler: IListenerType): IDisposable;
 }
 
 export interface IListenerType {

src/utils/Dom.ts

@@ -0,0 +1,26 @@
+import { IDisposable } from "../Interfaces";
+
+/**
+ * Adds a disposabe listener to a node in the DOM, returning the disposable.
+ * @param type The event type.
+ * @param handler The handler for the listener.
+ */
+export function addDisposableListener(
+  node: Element | Window | Document,
+  type: string,
+  handler: (e: any) => void,
+  useCapture?: boolean
+): IDisposable {
+  node.addEventListener(type, handler, useCapture);
+  return {
+    dispose: () => {
+      if (!handler) {
+        // Already disposed
+        return;
+      }
+      node.removeEventListener(type, handler, useCapture);
+      node = null;
+      handler = null;
+    }
+  };
+}