From 63ea4cb6e4f31a774ccf58d58e7c50b3e40fbe8a Mon Sep 17 00:00:00 2001
From: Robert Rees <robert.rees@guardian.co.uk>
Date: Sun, 13 Mar 2016 17:10:51 +0000
Subject: [PATCH 1/2] 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                             | 89 +++++++++++++++++++
 src/node.js                                   | 11 +++
 .../formatters/html/enforce-p-elements.js     |  4 +
 test/node.spec.js                             | 26 +++++-
 4 files changed, 129 insertions(+), 1 deletion(-)
 create mode 100644 examples/raw.html

diff --git a/examples/raw.html b/examples/raw.html
new file mode 100644
index 00000000..c0ab7c37
--- /dev/null
+++ b/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>
diff --git a/src/node.js b/src/node.js
index 26c3b68c..4c17fdf4 100644
--- a/src/node.js
+++ b/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,
diff --git a/src/plugins/core/formatters/html/enforce-p-elements.js b/src/plugins/core/formatters/html/enforce-p-elements.js
index 81fd49f5..3092767f 100644
--- a/src/plugins/core/formatters/html/enforce-p-elements.js
+++ b/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);
           })
diff --git a/test/node.spec.js b/test/node.spec.js
index 32e6d285..84c60f63 100644
--- a/test/node.spec.js
+++ b/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");
+
+      });
+    });
+  });
 });

From 48e7c37b7806f6375f3d92ef0ba0a2ee158db089 Mon Sep 17 00:00:00 2001
From: Robert Rees <robert.rees@guardian.co.uk>
Date: Sun, 13 Mar 2016 17:17:14 +0000
Subject: [PATCH 2/2] Restores the default content to the editor for
 consistency

---
 examples/raw.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/raw.html b/examples/raw.html
index c0ab7c37..f3f9f6bf 100644
--- a/examples/raw.html
+++ b/examples/raw.html
@@ -56,7 +56,7 @@
   var scribe = new Scribe(document.querySelector('.rte'));
   window.scribe = scribe;
 
-  //scribe.setContent('<p>Hello, World!<\/p>');
+  scribe.setContent('<p>Hello, World!<\/p>');
 
   scribe.use(scribePluginToolbar(document.querySelector('.rte-toolbar')));