From f516a4ff220a222b76a82b2016868f4d455a4b3d Mon Sep 17 00:00:00 2001
From: Justin Holdstock <justinholdstock91@gmail.com>
Date: Tue, 26 Sep 2017 13:03:46 -0700
Subject: [PATCH] Fix: Moved guards so drawing works when highlights disabled
 (#409)

---
 src/lib/annotations/doc/DocAnnotator.js       | 20 +++++++----
 .../doc/__tests__/DocAnnotator-test.js        | 36 ++++++++++++++++++-
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/src/lib/annotations/doc/DocAnnotator.js b/src/lib/annotations/doc/DocAnnotator.js
index f5cf659de..a6d1bfbb1 100644
--- a/src/lib/annotations/doc/DocAnnotator.js
+++ b/src/lib/annotations/doc/DocAnnotator.js
@@ -530,19 +530,27 @@ class DocAnnotator extends Annotator {
         this.annotatedElement.addEventListener('mouseup', this.highlightMouseupHandler);
 
         // Prevent all forms of highlight annotations if annotating (or plain AND comment highlights) is disabled
-        if (!this.permissions.canAnnotate || (!this.plainHighlightEnabled && !this.commentHighlightEnabled)) {
+        if (!this.permissions.canAnnotate) {
             return;
         }
 
         if (this.hasTouch && this.isMobile) {
-            document.addEventListener('selectionchange', this.onSelectionChange);
             this.annotatedElement.addEventListener('touchstart', this.drawingSelectionHandler);
+
+            // Highlight listeners
+            if (this.plainHighlightEnabled || this.commentHighlightEnabled) {
+                document.addEventListener('selectionchange', this.onSelectionChange);
+            }
         } else {
             this.annotatedElement.addEventListener('click', this.drawingSelectionHandler);
-            this.annotatedElement.addEventListener('dblclick', this.highlightMouseupHandler);
-            this.annotatedElement.addEventListener('mousedown', this.highlightMousedownHandler);
-            this.annotatedElement.addEventListener('contextmenu', this.highlightMousedownHandler);
-            this.annotatedElement.addEventListener('mousemove', this.getHighlightMouseMoveHandler());
+
+            // Highlight listeners
+            if (this.plainHighlightEnabled || this.commentHighlightEnabled) {
+                this.annotatedElement.addEventListener('dblclick', this.highlightMouseupHandler);
+                this.annotatedElement.addEventListener('mousedown', this.highlightMousedownHandler);
+                this.annotatedElement.addEventListener('contextmenu', this.highlightMousedownHandler);
+                this.annotatedElement.addEventListener('mousemove', this.getHighlightMouseMoveHandler());
+            }
         }
     }
 
diff --git a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js
index f26d78a46..72dd98096 100644
--- a/src/lib/annotations/doc/__tests__/DocAnnotator-test.js
+++ b/src/lib/annotations/doc/__tests__/DocAnnotator-test.js
@@ -660,7 +660,7 @@ describe('lib/annotations/doc/DocAnnotator', () => {
             annotator.bindDOMListeners();
         });
 
-        it('should bind selectionchange event, on the document, if on mobile and can annotate', () => {
+        it('should bind selectionchange and touchstart event, on the document, if on mobile and can annotate', () => {
             annotator.permissions.canAnnotate = true;
             annotator.isMobile = true;
             annotator.hasTouch = true;
@@ -673,6 +673,40 @@ describe('lib/annotations/doc/DocAnnotator', () => {
             expect(docListen).to.be.calledWith('selectionchange', sinon.match.func);
             expect(annotatedElementListen).to.be.calledWith('touchstart', sinon.match.func);
         });
+
+        it('should not bind selection change event if both annotation types are disabled, and touch enabled, mobile enabled', () => {
+            annotator.permissions.canAnnotate = true;
+            annotator.isMobile = true;
+            annotator.hasTouch = true;
+            annotator.plainHighlightEnabled = false;
+            annotator.commentHighlightEnabled = false;
+
+            const docListen = sandbox.spy(document, 'addEventListener');
+            const annotatedElementListen = sandbox.spy(annotator.annotatedElement, 'addEventListener');
+
+            annotator.bindDOMListeners();
+
+            expect(docListen).to.not.be.calledWith('selectionchange', sinon.match.func);
+            expect(annotatedElementListen).to.be.calledWith('touchstart', sinon.match.func);
+        });
+
+        it('should not bind selection change event if both annotation types are disabled, and touch disabled, mobile disabled', () => {
+            annotator.permissions.canAnnotate = true;
+            annotator.isMobile = false;
+            annotator.hasTouch = false;
+            annotator.plainHighlightEnabled = false;
+            annotator.commentHighlightEnabled = false;
+
+            stubs.elMock.expects('addEventListener').withArgs('mouseup', sinon.match.func);
+            stubs.elMock.expects('addEventListener').withArgs('click', sinon.match.func);
+            stubs.elMock.expects('addEventListener').never().withArgs('dblclick', sinon.match.func);
+            stubs.elMock.expects('addEventListener').never().withArgs('mousedown', sinon.match.func);
+            stubs.elMock.expects('addEventListener').never().withArgs('contextmenu', sinon.match.func);
+            stubs.elMock.expects('addEventListener').never().withArgs('mousemove', sinon.match.func);
+
+            annotator.bindDOMListeners();
+        });
+
     });
 
     describe('unbindDOMListeners()', () => {