scribe-editor - npm Package Compare versions

Comparing version 1.3.9 to 1.4.0

CHANGELOG.md

@@ -0,1 +1,7 @@
+# 1.4.0
+
+Changes the cleanup for Chrome inline style tags that happens in the patch for the `insertHTML` command. Previously span tags were aggressively stripped whereas now they are less aggressively removed to limit the fix just to the type of spans that Chrome inserts.
+
+Thanks [Christopher Liu](https://github.com/christopherliu) for contributing this change.
+
 # 1.3.9
@@ -2,0 +8,0 @@

package.json

 {
   "name": "scribe-editor",
-  "version": "1.3.9",
+  "version": "1.4.0",
   "main": "src/scribe.js",
@@ -5,0 +5,0 @@ "dependencies": {

README.md

@@ -141,3 +141,3 @@ Scribe [![Build Status](https://travis-ci.org/guardian/scribe.svg?branch=master)](https://travis-ci.org/guardian/scribe)
   <dt>Scribe Patches</dt>
-  <dd>Where patches for brower inconsistencies in native commands are defined.</dd>
+  <dd>Where patches for browser inconsistencies in native commands are defined.</dd>
   <dt>Native</dt>
@@ -144,0 +144,0 @@ </dl>

src/plugins/core/patches/commands/insert-html.js

 define([], function () {
-
-  'use strict';
-
+  "use strict";
   return function () {
@@ -14,2 +12,5 @@ return function (scribe) {
 
+          // TODO: share somehow with similar event patch for P nodes
+          removeChromeArtifacts(scribe.el);
+
           /**
@@ -20,37 +21,37 @@ * Chrome: If a parent node has a CSS `line-height` when we apply the
            * `line-height` on inline elements.
+           * 
            * As per: http://jsbin.com/ilEmudi/4/edit?css,js,output
-           *
-           * FIXME: what if the user actually wants to use SPANs? This could
-           * cause conflicts.
+           * More from the web: http://stackoverflow.com/q/15015019/40352
            */
-
-          // TODO: share somehow with similar event patch for P nodes
-          sanitize(scribe.el);
-
-          function sanitize(parentNode) {
-            var treeWalker = document.createTreeWalker(parentNode, NodeFilter.SHOW_ELEMENT, null, false);
-            var node = treeWalker.firstChild();
-            if (!node) { return; }
-
-            do {
-              if (node.nodeName === 'SPAN') {
-                element.unwrap(parentNode, node);
-              } else {
-                /**
-                 * If the list item contains inline elements such as
-                 * A, B, or I, Chrome will also append an inline style for
-                 * `line-height` on those elements, so we remove it here.
-                 */
-                node.style.lineHeight = null;
-
-                // There probably wasn’t a `style` attribute before, so
-                // remove it if it is now empty.
-                if (node.getAttribute('style') === '') {
-                  node.removeAttribute('style');
-                }
+          function removeChromeArtifacts(parentElement) {
+            // Can't use treeWalker: In at least Chrome, if a node is unwrapped,
+            // treeWalker.nextSibling will not work properly after that.
+            var childElement = parentElement.firstElementChild;
+            while (childElement) {
+              /**
+               * If the list item contains inline elements such as
+               * A, B, or I, Chrome will also append an inline style for
+               * `line-height` on those elements, so we remove it here.
+               */
+              var childStyle = window.getComputedStyle(childElement);
+              if ((childStyle.display === 'inline' || childElement.nodeName === 'SPAN') && window.getComputedStyle(parentElement)['line-height'] === childStyle['line-height']) {
+                childElement.style.lineHeight = null;
               }
-
-              // Sanitize children
-              sanitize(node);
-            } while ((node = treeWalker.nextSibling()));
+              
+              // We can discard an empty style attribute.
+              if (childElement.getAttribute('style') === '') {
+                childElement.removeAttribute('style');
+              }
+              
+              // Sanitize children.
+              removeChromeArtifacts(childElement);
+              
+              // We can discard an empty SPAN.
+              // (Don't do this until traversal's gone to the next element.)
+              var originalChild = childElement;
+              childElement = childElement.nextElementSibling;
+              if (originalChild.nodeName === 'SPAN' && originalChild.attributes.length === 0) {
+                element.unwrap(parentElement, originalChild);
+              }
+            }
           }
@@ -57,0 +58,0 @@ }.bind(this));

test/commands.spec.js

@@ -368,2 +368,47 @@ var chai = require('chai');
         });
+        
+        when('the command is executed with a value of "<p>1<b style="line-height: 2;">2</b><span style="line-height: 2;">3</span></p>"', function () {
+          beforeEach(function () {
+            // Focus it before-hand
+            scribeNode.click();
+
+            return executeCommand('insertHTML', '<p>1<b style="line-height: 2;">2</b><span style="line-height: 2;">3</span></p>');
+          });
+
+          it.skip('should remove line-height and unnecessary SPANs (only those generated by Chrome with specific line-height)', function () {
+            return scribeNode.getInnerHTML().then(function (innerHTML) {
+              expect(innerHTML).to.have.html('<p>1<b>2</b>3</p>');
+            });
+          });
+        });
+        
+        when('the command is executed with a value of "<p>1<b style="line-height: 2;">2</b><span style="line-height: 2;">3</span><span style="line-height: 2;">4</span></p>"', function () {
+          beforeEach(function () {
+            // Focus it before-hand
+            scribeNode.click();
+
+            return executeCommand('insertHTML', '<p>1<b style="line-height: 2;">2</b><span style="line-height: 2;">3</span><span style="line-height: 2;">4</span></p>');
+          });
+
+          it.skip('should remove unnecessary Chrome auto-generated line-height elements even when they are not the first items in a subtree', function () {
+            return scribeNode.getInnerHTML().then(function (innerHTML) {
+              expect(innerHTML).to.have.html('<p>1<b>2</b>34</p>');
+            });
+          });
+        });
+        
+        when('the command is executed with a value of "<p><span style="color:green;">1</span></p>"', function () {
+          beforeEach(function () {
+            // Focus it before-hand
+            scribeNode.click();
+
+            return executeCommand('insertHTML', '<p><span style="color:green;">1</span></p>');
+          });
+
+          it.skip('should not remove SPANs when they have attributes besides line-height (and are thus not Chrome auto-generated)', function () {
+            return scribeNode.getInnerHTML().then(function (innerHTML) {
+              expect(innerHTML).to.have.html('<p><span style="color:green;">1</span></p>');
+            });
+          });
+        });
       });
@@ -370,0 +415,0 @@

No alert changes

Improved metrics

Total package byte prevSize

203132

increased by1.58%

Lines of code

4215

increased by0.96%

No dependency changes