Merge pull request #522 from guardian/rahb/fix-scribe-commands

Fix Scribe adding links to other contenteditables
guardian · Aug 10, 2018 · f56eda1 · f56eda1
2 parents 52e78fa + 84a4a75
commit f56eda1
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 21 deletions.
diff --git a/src/api/selection.js b/src/api/selection.js
@@ -101,35 +101,38 @@ define(function () {
         nodeHelpers.getAncestor(node, scribe.el, nodeFilter);
     };
 
+    Selection.prototype.isInScribe = function () {
+      var range = this.range;
+      return range
+        //we need to ensure that the scribe's element lives within the current document to avoid errors with the range comparison (see below)
+        //one way to do this is to check if it's visible (is this the best way?).
+        && document.contains(scribe.el)
+        //we want to ensure that the current selection is within the current scribe node
+        //if this isn't true scribe will place markers within the selections parent
+        //we want to ensure that scribe ONLY places markers within it's own element
+        && scribe.el.contains(range.startContainer)
+        && scribe.el.contains(range.endContainer);
+    }
+
     Selection.prototype.placeMarkers = function () {
       var range = this.range;
-      if (!range) {
-        return;
-      }
 
-      //we need to ensure that the scribe's element lives within the current document to avoid errors with the range comparison (see below)
-      //one way to do this is to check if it's visible (is this the best way?).
-      if (!document.contains(scribe.el)) {
+      if (!this.isInScribe()) {
         return;
       }
 
-      //we want to ensure that the current selection is within the current scribe node
-      //if this isn't true scribe will place markers within the selections parent
-      //we want to ensure that scribe ONLY places markers within it's own element
-      if (scribe.el.contains(range.startContainer) && scribe.el.contains(range.endContainer)) {
-        // insert start marker
-        insertMarker(range.cloneRange(), createMarker());
-
-        if (! range.collapsed ) {
-          // End marker
-          var rangeEnd = range.cloneRange();
-          rangeEnd.collapse(false);
-          insertMarker(rangeEnd, createMarker());
-        }
+      // insert start marker
+      insertMarker(range.cloneRange(), createMarker());
 
-        this.selection.removeAllRanges();
-        this.selection.addRange(range);
+      if (! range.collapsed ) {
+        // End marker
+        var rangeEnd = range.cloneRange();
+        rangeEnd.collapse(false);
+        insertMarker(rangeEnd, createMarker());
       }
+
+      this.selection.removeAllRanges();
+      this.selection.addRange(range);
     };
 
     Selection.prototype.getMarkers = function () {

diff --git a/src/plugins/core/patches/commands/create-link.js b/src/plugins/core/patches/commands/create-link.js
@@ -10,6 +10,14 @@ define(function () {
       createLinkCommand.execute = function (value) {
         var selection = new scribe.api.Selection();
 
+        /**
+         * make sure we're not touching any none Scribe elements
+         * in the page
+         */
+        if (!selection.isInScribe()) {
+          return;
+        }
+
         /**
          * Firefox does not create a link when selection is collapsed
          * so we create it manually. http://jsbin.com/tutufi/2/edit?js,output