Fix: Only unbind listeners from existing DOM elements (#321)

* Fix: Don't unbind listeners from non-existent elements * Chore: Cleanup * Chore: Standardize guard clauses * Chore: Guarding the removal of additional event listener
box · Jan 15, 2019 · 6a71a9e · 6a71a9e
1 parent a474251
commit 6a71a9e
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 22 deletions.
diff --git a/src/AnnotationThread.js b/src/AnnotationThread.js
@@ -454,11 +454,9 @@ class AnnotationThread extends EventEmitter {
      * @return {void}
      */
     unbindDOMListeners() {
-        if (!this.element) {
-            return;
+        if (this.element) {
+            this.element.removeEventListener('click', this.renderAnnotationPopover);
         }
-
-        this.element.removeEventListener('click', this.renderAnnotationPopover);
     }
 
     /**

diff --git a/src/Annotator.js b/src/Annotator.js
@@ -343,7 +343,10 @@ class Annotator extends EventEmitter {
      */
     unbindCustomListeners() {
         this.removeListener(ANNOTATOR_EVENT.scale, this.scaleAnnotations);
-        this.api.removeListener(ANNOTATOR_EVENT.error, this.handleServicesErrors);
+
+        if (this.api) {
+            this.api.removeListener(ANNOTATOR_EVENT.error, this.handleServicesErrors);
+        }
     }
 
     /**

diff --git a/src/controllers/AnnotationModeController.js b/src/controllers/AnnotationModeController.js
@@ -140,7 +140,10 @@ class AnnotationModeController extends EventEmitter {
         if (this.buttonEl) {
             this.buttonEl.removeEventListener('click', this.toggleMode);
         }
-        this.api.removeListener(CONTROLLER_EVENT.error, this.handleAPIErrors);
+
+        if (this.api) {
+            this.api.removeListener(CONTROLLER_EVENT.error, this.handleAPIErrors);
+        }
     }
 
     /**

diff --git a/src/doc/DocAnnotator.js b/src/doc/DocAnnotator.js
@@ -364,10 +364,19 @@ class DocAnnotator extends Annotator {
     unbindDOMListeners() {
         super.unbindDOMListeners();
 
-        this.container.removeEventListener('resize', this.resetAnnotationUI);
-        this.annotatedElement.removeEventListener('wheel', this.hideCreateDialog);
-        this.annotatedElement.removeEventListener('touchend', this.hideCreateDialog);
-        this.annotatedElement.removeEventListener('click', this.clickHandler);
+        if (this.container) {
+            this.container.removeEventListener('resize', this.resetAnnotationUI);
+        }
+
+        if (this.annotatedElement) {
+            this.annotatedElement.removeEventListener('wheel', this.hideCreateDialog);
+            this.annotatedElement.removeEventListener('touchend', this.hideCreateDialog);
+            this.annotatedElement.removeEventListener('click', this.clickHandler);
+            this.annotatedElement.removeEventListener('mouseup', this.highlightMouseupHandler);
+            this.annotatedElement.removeEventListener('dblclick', this.highlightMouseupHandler);
+            this.annotatedElement.removeEventListener('mousedown', this.highlightMousedownHandler);
+            this.annotatedElement.removeEventListener('contextmenu', this.highlightMousedownHandler);
+        }
 
         if (this.highlightThrottleHandle) {
             cancelAnimationFrame(this.highlightThrottleHandle);
@@ -381,11 +390,6 @@ class DocAnnotator extends Annotator {
 
         if (this.hasTouch) {
             document.removeEventListener('selectionchange', this.onSelectionChange);
-        } else {
-            this.annotatedElement.removeEventListener('mouseup', this.highlightMouseupHandler);
-            this.annotatedElement.removeEventListener('dblclick', this.highlightMouseupHandler);
-            this.annotatedElement.removeEventListener('mousedown', this.highlightMousedownHandler);
-            this.annotatedElement.removeEventListener('contextmenu', this.highlightMousedownHandler);
         }
     }
 

diff --git a/src/drawing/DrawingThread.js b/src/drawing/DrawingThread.js
@@ -154,11 +154,13 @@ class DrawingThread extends AnnotationThread {
      * @return {void}
      */
     unbindDrawingListeners() {
-        this.annotatedElement.removeEventListener('mousemove', this.userMoveHandler);
-        this.annotatedElement.removeEventListener('mouseup', this.userStopHandler);
-        this.annotatedElement.removeEventListener('touchmove', this.userMoveHandler);
-        this.annotatedElement.removeEventListener('touchcancel', this.userStopHandler);
-        this.annotatedElement.removeEventListener('touchend', this.userStopHandler);
+        if (this.annotatedElement) {
+            this.annotatedElement.removeEventListener('mousemove', this.userMoveHandler);
+            this.annotatedElement.removeEventListener('mouseup', this.userStopHandler);
+            this.annotatedElement.removeEventListener('touchmove', this.userMoveHandler);
+            this.annotatedElement.removeEventListener('touchcancel', this.userStopHandler);
+            this.annotatedElement.removeEventListener('touchend', this.userStopHandler);
+        }
     }
 
     /**

diff --git a/src/image/ImageAnnotator.js b/src/image/ImageAnnotator.js
@@ -117,13 +117,17 @@ class ImageAnnotator extends Annotator {
 
     /** @inheritdoc */
     bindDOMListeners() {
-        this.annotatedElement.addEventListener('mouseup', this.hideAnnotations);
+        if (this.annotatedElement) {
+            this.annotatedElement.addEventListener('mouseup', this.hideAnnotations);
+        }
         super.bindDOMListeners();
     }
 
     /** @inheritdoc */
     unbindDOMListeners() {
-        this.annotatedElement.removeEventListener('mouseup', this.hideAnnotations);
+        if (this.annotatedElement) {
+            this.annotatedElement.removeEventListener('mouseup', this.hideAnnotations);
+        }
         super.bindDOMListeners();
     }
 }