Builtin-plugins: stop wrapping whitespace text nodes

Issue #456 revealed some strange behaviour that is normally masked by the use of the Sanitizer plugin (hence the addition of a test page that is minimal in terms of plugins). This turned out to be because we were wrapping text nodes that had no content.

To fix the issue these nodes are currently ignore in the wrapping process but it would probably be better to delete them from the content.

The easiest way I've found to have this be repeatable is to change the default content to contain some indented paragraphs. If you then bold any word in the content all the whitespace becomes wrapped in a paragraph creating noticeable changes to the visual spacing.

examples/raw.html

@@ -0,0 +1,89 @@
+<!--
+This example demonstrates how to consume the Scribe
+editor using RequireJS and the AMD module syntax.
+
+Note that you'll need to install scribe's dependencies through
+`bower install`. See http://bower.io/ if you are unfamiliar.
+-->
+<style>
+  button {
+    height: 3em;
+  }
+
+  .active {
+    border-style: inset;
+  }
+
+  .rte, .rte-toolbar {
+    display: block;
+  }
+
+  p {
+    margin-top: 0;
+  }
+
+  .rte {
+    border: 1px solid gray;
+    height: 300px;
+    overflow: auto;
+  }
+  .rte-output {
+    width: 100%;
+    height: 10em;
+  }
+
+  .raw-content-editable {
+    width: 100%;
+    min-height: 2rem;
+    border: solid 1px lightgray;
+  }
+</style>
+<script src="../bower_components/requirejs/require.js"></script>
+<script>
+require({
+  paths: {
+    'lodash-amd': '../bower_components/lodash-amd',
+    'immutable': '../bower_components/immutable/dist/immutable'
+  }
+}, [
+  '../src/scribe',
+  '../bower_components/scribe-plugin-toolbar/src/scribe-plugin-toolbar',
+  '../bower_components/scribe-plugin-formatter-plain-text-convert-new-lines-to-html/src/scribe-plugin-formatter-plain-text-convert-new-lines-to-html',
+], function (
+  Scribe,
+  scribePluginToolbar
+) {
+  var scribe = new Scribe(document.querySelector('.rte'));
+  window.scribe = scribe;
+
+  //scribe.setContent('<p>Hello, World!<\/p>');
+
+  scribe.use(scribePluginToolbar(document.querySelector('.rte-toolbar')));
+
+  scribe.on('content-changed', updateHtml);
+
+  function updateHtml() {
+    document.querySelector('.rte-output').value = scribe.getHTML();
+  }
+
+  updateHtml();
+});
+</script>
+<div class="rte-toolbar">
+  <button data-command-name="bold">Bold</button>
+  <button data-command-name="italic">Italic</button>
+  <button data-command-name="h2">H2</button>
+</div>
+<div class="rte">
+</div>
+
+<section>
+  <h1>Output</h1>
+  <textarea class="rte-output" readonly></textarea>
+</section>
+
+<section>
+  <h1>Basic content-editable</h1>
+
+  <div class="raw-content-editable" contenteditable="true"></div>
+</section>

src/node.js

@@ -56,6 +56,16 @@ define([
     return elementHasClass(Node, 'scribe-not-observable')(node);
   }
 
+  function isWhitespaceOnlyTextNode(Node, node) {
+    if(node.nodeType === Node.TEXT_NODE
+      && /^\s*$/.test(node.nodeValue)) {
+      return true;
+    }
+
+    return false;
+
+  }
+
   function firstDeepestChild(node) {
     var fs = node.firstChild;
     return !fs || fs.nodeName === 'BR' ?
@@ -158,6 +168,7 @@ define([
     isEmptyInlineElement: isEmptyInlineElement,
     isText: isText,
     isEmptyTextNode: isEmptyTextNode,
+    isWhitespaceOnlyTextNode: isWhitespaceOnlyTextNode,
     isFragment: isFragment,
     isBefore: isBefore,
     isSelectionMarkerNode: isSelectionMarkerNode,

src/plugins/core/formatters/html/enforce-p-elements.js

@@ -28,7 +28,11 @@ define([
        */
       function wrapChildNodes(parentNode) {
         var index = 0;
+
         Immutable.List(parentNode.childNodes)
+          .filterNot(function(node) {
+            return nodeHelpers.isWhitespaceOnlyTextNode(Node, node);
+          })
           .filter(function(node) {
             return node.nodeType === Node.TEXT_NODE || !nodeHelpers.isBlockElement(node);
           })

test/node.spec.js

@@ -17,7 +17,8 @@ var fakeBrowser = new MockBrowser();
 var doc = fakeBrowser.getDocument();
 
 var FakeNode = {
-  ELEMENT_NODE: 1
+  ELEMENT_NODE: 1,
+  TEXT_NODE: 3
 };
 
 describe('Node type checking', function() {
@@ -46,4 +47,27 @@ describe('Node type checking', function() {
       assert.isFalse(checkFunction(fakeElement));
     });
   });
+
+  describe('text nodes', function() {
+    describe('that are whitespace-only', function() {
+      it('are detected', function() {
+        var emptyTextNode = {
+          nodeValue: "   ",
+          nodeType: 3
+        }
+
+        assert.isTrue(nodeHelpers.isWhitespaceOnlyTextNode(FakeNode, emptyTextNode), "Whitespace-only node not detected correctly");
+      });
+
+      it('are not falsely identified', function() {
+        var testNode = {
+          nodeValue: "hello world",
+          nodeType: 3
+        }
+
+        assert.isFalse(nodeHelpers.isWhitespaceOnlyTextNode(FakeNode, testNode), "Regular text node identified as whitespace-only");
+
+      });
+    });
+  });
 });