Fix: Don't clear selection if selecting outside of annotated element (#…

…298)
box · Nov 28, 2018 · 5e277a0 · 5e277a0
1 parent 873394f
commit 5e277a0
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 2 deletions.
diff --git a/src/doc/DocAnnotator.js b/src/doc/DocAnnotator.js
@@ -533,6 +533,19 @@ class DocAnnotator extends Annotator {
      * @return {void}
      */
     onSelectionChange(event: Event) {
+        const selection = window.getSelection();
+        const selectionNode = selection.anchorNode;
+        // If the selection is not contained within the annotated element, don't do anything to avoid conflicting
+        // with default selection behavior outside of annotations.
+        // In IE 11, we need to check the parent of the selection, as the anchor node is a TextElement that cannot
+        // be found by it's parent divs.
+        if (
+            !this.annotatedElement.contains(selectionNode) ||
+            !this.annotatedElement.contains(selectionNode.parentNode)
+        ) {
+            return;
+        }
+
         event.preventDefault();
         event.stopPropagation();
 
@@ -542,7 +555,6 @@ class DocAnnotator extends Annotator {
         }
 
         // Bail if mid highlight and tapping on the screen
-        const selection = window.getSelection();
         const isClickOutsideCreateDialog = this.isCreatingHighlight && util.isInDialog(event);
         if (!docUtil.isValidSelection(selection) || isClickOutsideCreateDialog) {
             this.lastHighlightEvent = null;

diff --git a/src/doc/__tests__/DocAnnotator-test.js b/src/doc/__tests__/DocAnnotator-test.js
@@ -802,18 +802,34 @@ describe('doc/DocAnnotator', () => {
                 stopPropagation: jest.fn()
             };
 
+            annotator.annotatedElement = {
+                contains: jest.fn().mockReturnValue(true)
+            };
+
             docUtil.isValidSelection = jest.fn().mockReturnValue(true);
             annotator.lastSelection = {};
 
             annotator.highlighter = { removeAllHighlights: jest.fn() };
             annotator.resetHighlightSelection = jest.fn();
 
-            window.getSelection = jest.fn();
+            window.getSelection = jest.fn().mockReturnValue({
+                anchorNode: {
+                    parentNode: 'Node'
+                }
+            });
             util.getPageInfo = jest.fn().mockReturnValue({ page: 1 });
             util.findClosestElWithClass = jest.fn().mockReturnValue(null);
             annotator.isCreatingHighlight = false;
         });
 
+        it('should do nothing if the selection is not contained in the annotatedElement', () => {
+            annotator.annotatedElement.contains.mockReturnValue(false);
+            annotator.onSelectionChange(event);
+
+            expect(event.preventDefault).not.toBeCalled();
+            expect(event.stopPropagation).not.toBeCalled();
+        });
+
         it('should reset the selectionEndTimeout', () => {
             annotator.selectionEndTimeout = 1;
             docUtil.isValidSelection = jest.fn().mockReturnValue(false);
@@ -862,6 +878,9 @@ describe('doc/DocAnnotator', () => {
             const selection = {
                 rangeCount: 10,
                 isCollapsed: false,
+                anchorNode: {
+                    parentNode: 'Node'
+                },
                 toString: () => 'asdf'
             };
             window.getSelection = jest.fn().mockReturnValue(selection);
@@ -878,6 +897,9 @@ describe('doc/DocAnnotator', () => {
             const selection = {
                 rangeCount: 10,
                 isCollapsed: false,
+                anchorNode: {
+                    parentNode: 'Node'
+                },
                 toString: jest.fn().mockReturnValue('asdf')
             };
             annotator.modeControllers = {