Fix: Only bind custom listeners when annotation threads exist (#74)

box · Apr 13, 2017 · 1e2e9f7 · 1e2e9f7
1 parent 5daa11e
commit 1e2e9f7
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 6 deletions.
diff --git a/src/lib/annotations/Annotator.js b/src/lib/annotations/Annotator.js
@@ -255,7 +255,9 @@ class Annotator extends EventEmitter {
 
                     // Bind events on valid annotation thread
                     const thread = this.createAnnotationThread(annotations, firstAnnotation.location, firstAnnotation.type);
-                    this.bindCustomListenersOnThread(thread);
+                    if (thread) {
+                        this.bindCustomListenersOnThread(thread);
+                    }
                 });
             });
     }
@@ -421,10 +423,13 @@ class Annotator extends EventEmitter {
 
         // Create new thread with no annotations, show indicator, and show dialog
         const thread = this.createAnnotationThread([], location, constants.ANNOTATION_TYPE_POINT);
-        thread.show();
 
-        // Bind events on thread
-        this.bindCustomListenersOnThread(thread);
+        if (thread) {
+            thread.show();
+
+            // Bind events on thread
+            this.bindCustomListenersOnThread(thread);
+        }
     }
 
     /**

diff --git a/src/lib/annotations/__tests__/Annotator-test.js b/src/lib/annotations/__tests__/Annotator-test.js
@@ -252,15 +252,17 @@ describe('lib/annotations/Annotator', () => {
         });
 
         it('should reset and create a new thread map by fetching annotation data from the server', () => {
-            sandbox.stub(annotator, 'createAnnotationThread').returns(stubs.thread);
+            stubs.createThread = sandbox.stub(annotator, 'createAnnotationThread');
+            stubs.createThread.onFirstCall();
+            stubs.createThread.onSecondCall().returns(stubs.thread);
             sandbox.stub(annotator, 'bindCustomListenersOnThread');
 
             const result = annotator.fetchAnnotations();
 
             return stubs.threadPromise.then(() => {
                 expect(Object.keys(annotator._threads).length === 0).to.be.true;
                 expect(annotator.createAnnotationThread).to.be.calledTwice;
-                expect(annotator.bindCustomListenersOnThread).to.be.calledTwice;
+                expect(annotator.bindCustomListenersOnThread).to.be.calledOnce;
                 expect(result).to.be.an.object;
             });
         });
@@ -373,6 +375,17 @@ describe('lib/annotations/Annotator', () => {
             expect(annotator.togglePointModeHandler).to.not.be.called;
         });
 
+        it('should not do anything if thread is invalid', () => {
+            stubs.destroy.returns(false);
+
+            stubs.threadMock.expects('show').never();
+            annotator.pointClickHandler(event);
+
+            expect(annotator.getLocationFromEvent).to.be.called;
+            expect(annotator.togglePointModeHandler).to.be.called;
+            expect(annotator.bindCustomListenersOnThread).to.not.be.called;
+        });
+
         it('should not create a thread if a location object cannot be inferred from the event', () => {
             stubs.destroy.returns(false);
             stubs.getLocation.returns(null);