Merge pull request #281 from mixonic/yb-fix-bug-276

Rebase of #227
bustle · Jan 5, 2016 · 96ea818 · 96ea818
2 parents 1754844 + b3aac9e
commit 96ea818
Show file tree

Hide file tree

Showing 7 changed files with 134 additions and 25 deletions.
diff --git a/src/js/parsers/dom.js b/src/js/parsers/dom.js
@@ -9,11 +9,12 @@ import {
 } from '../models/types';
 import {
   isTextNode,
+  isCommentNode,
   normalizeTagName
 } from '../utils/dom-utils';
 import {
   detect,
-  forEach,
+  forEach
 } from '../utils/array-utils';
 import { TAB } from 'mobiledoc-kit/utils/characters';
 
@@ -34,6 +35,7 @@ export function transformHTMLText(textContent) {
 
 function isGoogleDocsContainer(element) {
   return !isTextNode(element) &&
+         !isCommentNode(element) &&
          normalizeTagName(element.tagName) === normalizeTagName('b') &&
          GOOGLE_DOCS_CONTAINER_ID_REGEX.test(element.id);
 }

diff --git a/src/js/parsers/section.js b/src/js/parsers/section.js
@@ -1,6 +1,3 @@
-const TEXT_NODE = 3;
-const ELEMENT_NODE = 1;
-
 import {
   DEFAULT_TAG_NAME,
   VALID_MARKUP_SECTION_TAGNAMES
@@ -27,7 +24,9 @@ import {
 import {
   getAttributes,
   normalizeTagName,
-  isTextNode
+  isTextNode,
+  isCommentNode,
+  NODE_TYPES
 } from 'mobiledoc-kit/utils/dom-utils';
 
 import {
@@ -135,10 +134,10 @@ export default class SectionParser {
     }
 
     switch (node.nodeType) {
-      case TEXT_NODE:
+      case NODE_TYPES.TEXT:
         this.parseTextNode(node);
         break;
-      case ELEMENT_NODE:
+      case NODE_TYPES.ELEMENT:
         this.parseElementNode(node);
         break;
       default:
@@ -300,8 +299,9 @@ export default class SectionParser {
   }
 
   _isSkippable(element) {
-    return element.nodeType === ELEMENT_NODE &&
-           contains(SKIPPABLE_ELEMENT_TAG_NAMES,
-                    normalizeTagName(element.tagName));
+    return isCommentNode(element) ||
+           (element.nodeType === NODE_TYPES.ELEMENT &&
+            contains(SKIPPABLE_ELEMENT_TAG_NAMES,
+                    normalizeTagName(element.tagName)));
   }
 }
diff --git a/src/js/utils/dom-utils.js b/src/js/utils/dom-utils.js
@@ -1,9 +1,17 @@
 import { forEach } from './array-utils';
 
-const TEXT_NODE_TYPE = 3;
+export const NODE_TYPES = {
+  ELEMENT: 1,
+  TEXT: 3,
+  COMMENT: 8
+};
 
 function isTextNode(node) {
-  return node.nodeType === TEXT_NODE_TYPE;
+  return node.nodeType === NODE_TYPES.TEXT;
+}
+
+function isCommentNode(node) {
+  return node.nodeType === NODE_TYPES.COMMENT;
 }
 
 // perform a pre-order tree traversal of the dom, calling `callbackFn(node)`
@@ -112,6 +120,7 @@ export {
   addClassName,
   normalizeTagName,
   isTextNode,
+  isCommentNode,
   parseHTML,
   findOffsetInElement
 };
diff --git a/tests/fixtures/google-docs.js b/tests/fixtures/google-docs.js
diff --git a/tests/unit/parsers/html-google-sheets-test.js b/tests/unit/parsers/html-google-sheets-test.js
@@ -0,0 +1,86 @@
+import HTMLParser from 'mobiledoc-kit/parsers/html';
+import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
+import Helpers from '../../test-helpers';
+
+const {module, test} = Helpers;
+
+let parser;
+
+module('Unit: Parser: HTMLParser Google Sheets', {
+  beforeEach() {
+    const options = {};
+    const builder = new PostNodeBuilder();
+    parser = new HTMLParser(builder, options);
+  },
+  afterEach() {
+    parser = null;
+  }
+});
+
+// No formatting
+test('#parse returns a markup section when given a cell without formatting', (assert) => {
+  const text = `<meta http-equiv="content-type" content="text/html; charset=utf-8"><style type="text/css"><!--td {border: 1px solid #ccc;}br {mso-data-placement:same-cell;}--></style><span style="font-size:13px;font-family:Arial;" data-sheets-value="[null,2,&quot;Ways of climbing over the wall&quot;]" data-sheets-userformat="[null,null,513,[null,0],null,null,null,null,null,null,null,null,0]">Ways of climbing over the wall</span>`;
+  const post = parser.parse(text);
+  const expected = Helpers.postAbstract.build(({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker('Ways of climbing over the wall')])]);
+  });
+
+  assert.postIsSimilar(post, expected);
+});
+
+// No formatting (Chrome - Windows)
+test('#parse returns a markup section when given a cell without formatting (Chrome - Windows)', (assert) => {
+  const text = `<html><body><!--StartFragment--><style type="text/css"><!--td {border: 1px solid #ccc;}br {mso-data-placement:same-cell;}--></style><span style="font-size:13px;font-family:Arial;" data-sheets-value="[null,2,&quot;Ways of climbing over the wall&quot;]" data-sheets-userformat="[null,null,513,[null,0],null,null,null,null,null,null,null,null,0]">Ways of climbing over the wall</span><!--EndFragment--></body></html>`;
+  const post = parser.parse(text);
+  const expected = Helpers.postAbstract.build(({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker('Ways of climbing over the wall')])]);
+  });
+
+  assert.postIsSimilar(post, expected);
+});
+
+// Cell in bold
+test('#parse returns a markup section with bold when given a cell in bold', (assert) => {
+  const text = `<meta http-equiv="content-type" content="text/html; charset=utf-8"><style type="text/css"><!--td {border: 1px solid #ccc;}br {mso-data-placement:same-cell;}--></style><span style="font-size:13px;font-family:Arial;font-weight:bold;" data-sheets-value="[null,2,&quot;Ways of climbing over the wall&quot;]" data-sheets-userformat="[null,null,16897,[null,0],null,null,null,null,null,null,null,null,0,null,null,null,null,1]">Ways of climbing over the wall</span>`;
+  const post = parser.parse(text);
+  const expected = Helpers.postAbstract.build(({post, markupSection, marker, markup}) => {
+    const b = markup('strong');
+    return post([markupSection('p', [marker('Ways of climbing over the wall', [b])])]);
+  });
+
+  assert.postIsSimilar(post, expected);
+});
+
+// Cell in bold (Chrome - Windows)
+test('#parse returns a markup section with bold when given a cell in bold (Chrome - Windows)', (assert) => {
+  const text = `<html><body><!--StartFragment--><style type="text/css"><!--td {border: 1px solid #ccc;}br {mso-data-placement:same-cell;}--></style><span style="font-size:13px;font-family:Arial;font-weight:bold;" data-sheets-value="[null,2,&quot;Ways of climbing over the wall&quot;]" data-sheets-userformat="[null,null,16897,[null,0],null,null,null,null,null,null,null,null,0,null,null,null,null,1]">Ways of climbing over the wall</span><!--EndFragment--></body></html>`;
+  const post = parser.parse(text);
+  const expected = Helpers.postAbstract.build(({post, markupSection, marker, markup}) => {
+    const b = markup('strong');
+    return post([markupSection('p', [marker('Ways of climbing over the wall', [b])])]);
+  });
+
+  assert.postIsSimilar(post, expected);
+});
+
+// Two adjacent cells without formatting
+test('#parse returns a single markup section when given two cells on top of each other without formatting', (assert) => {
+  const text = `<meta http-equiv="content-type" content="text/html; charset=utf-8"><meta name="generator" content="Sheets"><style type="text/css"><!--td {border: 1px solid #ccc;}br {mso-data-placement:same-cell;}--></style><table cellspacing="0" cellpadding="0" dir="ltr" border="1" style="table-layout:fixed;font-size:13px;font-family:arial,sans,sans-serif;border-collapse:collapse;border:1px solid #ccc"><colgroup><col width="361"></colgroup><tbody><tr style="height:21px;"><td style="padding:2px 3px 2px 3px;vertical-align:bottom;font-family:Arial;" data-sheets-value="[null,2,&quot;Ostalgia&quot;]">Ostalgia</td></tr><tr style="height:21px;"><td style="padding:2px 3px 2px 3px;vertical-align:bottom;font-family:Arial;" data-sheets-value="[null,2,&quot;Photo&quot;]">Photo</td></tr></tbody></table>`;
+  const post = parser.parse(text);
+  const expected = Helpers.postAbstract.build(({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker('OstalgiaPhoto')])]);
+  });
+
+  assert.postIsSimilar(post, expected);
+});
+
+// Two adjacent cells without formatting (Chrome - Windows)
+test('#parse returns a single markup section when given two cells on top of each other without formatting (Chrome - Windows)', (assert) => {
+  const text = `<html><body><!--StartFragment--><meta name="generator" content="Sheets"><style type="text/css"><!--td {border: 1px solid #ccc;}br {mso-data-placement:same-cell;}--></style><table cellspacing="0" cellpadding="0" dir="ltr" border="1" style="table-layout:fixed;font-size:13px;font-family:arial,sans,sans-serif;border-collapse:collapse;border:1px solid #ccc"><colgroup><col width="361"></colgroup><tbody><tr style="height:21px;"><td style="padding:2px 3px 2px 3px;vertical-align:bottom;font-family:Arial;" data-sheets-value="[null,2,&quot;Ostalgia&quot;]">Ostalgia</td></tr><tr style="height:21px;"><td style="padding:2px 3px 2px 3px;vertical-align:bottom;font-family:Arial;" data-sheets-value="[null,2,&quot;Photo&quot;]">Photo</td></tr></tbody></table><!--EndFragment--></body></html>`;
+  const post = parser.parse(text);
+  const expected = Helpers.postAbstract.build(({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker('OstalgiaPhoto')])]);
+  });
+
+  assert.postIsSimilar(post, expected);
+});
diff --git a/tests/unit/parsers/section-test.js b/tests/unit/parsers/section-test.js
@@ -161,3 +161,13 @@ test('#parse skips STYLE nodes', (assert) => {
 
   assert.equal(sections.length, 0, 'does not parse style');
 });
+
+test('#parse skips Comment nodes', (assert) => {
+  let element = buildDOM(`
+    <!--Some comment-->
+  `).firstChild;
+  parser = new SectionParser(builder);
+  let sections = parser.parse(element);
+
+  assert.equal(sections.length, 0, 'does not parse comments');
+});
diff --git a/tests/unit/parsers/text-test.js b/tests/unit/parsers/text-test.js
@@ -5,21 +5,15 @@ import Helpers from '../../test-helpers';
 
 const {module, test} = Helpers;
 
-let editorElement, builder, parser, editor;
+let parser;
 
 module('Unit: Parser: TextParser', {
   beforeEach() {
-    editorElement = $('#editor')[0];
-    builder = new PostNodeBuilder();
+    const builder = new PostNodeBuilder();
     parser = new TextParser(builder);
   },
   afterEach() {
-    builder = null;
     parser = null;
-    if (editor) {
-      editor.destroy();
-      editor = null;
-    }
   }
 });
 
@@ -77,7 +71,7 @@ test('#parse returns multiple sections when lines are separated by CR', (assert)
 
 test('#parse returns list section when text starts with "*"', (assert) => {
   let text = '* a list item';
-  
+
   let post = parser.parse(text);
   let expected = Helpers.postAbstract.build(({post, listSection, listItem, marker}) => {
     return post([
@@ -90,7 +84,7 @@ test('#parse returns list section when text starts with "*"', (assert) => {
 
 test('#parse returns list section with multiple items when text starts with "*"', (assert) => {
   let text = ['* first', '* second'].join(SECTION_BREAK);
-  
+
   let post = parser.parse(text);
   let expected = Helpers.postAbstract.build(({post, listSection, listItem, marker}) => {
     return post([
@@ -106,7 +100,7 @@ test('#parse returns list section with multiple items when text starts with "*"'
 
 test('#parse returns list sections separated by markup sections', (assert) => {
   let text = ['* first list', 'middle section', '* second list'].join(SECTION_BREAK);
-  
+
   let post = parser.parse(text);
   let expected = Helpers.postAbstract.build(
     ({post, listSection, listItem, markupSection, marker}) => {
@@ -126,7 +120,7 @@ test('#parse returns list sections separated by markup sections', (assert) => {
 
 test('#parse returns ordered list items', (assert) => {
   let text = '1. first list';
-  
+
   let post = parser.parse(text);
   let expected = Helpers.postAbstract.build(
     ({post, listSection, listItem, markupSection, marker}) => {
@@ -138,7 +132,7 @@ test('#parse returns ordered list items', (assert) => {
 
 test('#parse can have ordered and unordered lists together', (assert) => {
   let text = ['1. ordered list', '* unordered list'].join(SECTION_BREAK);
-  
+
   let post = parser.parse(text);
   let expected = Helpers.postAbstract.build(
     ({post, listSection, listItem, markupSection, marker}) => {