🐛 Fixed parser plugin handling of top-level comment nodes

no issue

- the section parser allows comment nodes to be parsed by parser plugins but due to top-level comment nodes being skipped it meant that they wouldn't be seen in certain circumstances
- removes the top-level skip of comment nodes so that they will be passed through the parser plugins
- adds specific skips for comment nodes in areas that create/update state from elements
- allows for `addMarkerable` to be called by a parser plugin when no state has been set up yet such as in the case where the first element in a parsed section is a comment node that is handled by a parser plugin

Author: kevinansfield

src/js/parsers/section.js

@@ -98,7 +98,7 @@ class SectionParser {
       addSection: (section) => {
         // avoid creating empty paragraphs due to wrapper elements around
         // parser-plugin-handled elements
-        if (this.state.section.isMarkerable && !this.state.text && !this.state.section.text) {
+        if (this.state.section && this.state.section.isMarkerable && !this.state.section.text && !this.state.text) {
           this.state.section = null;
         } else {
           this._closeCurrentSection();
@@ -108,6 +108,13 @@ class SectionParser {
       addMarkerable: (marker) => {
         let { state } = this;
         let { section } = state;
+        // if the first element doesn't create it's own state and it's plugin
+        // handler uses `addMarkerable` we won't have a section yet
+        if (!section) {
+          state.text = '';
+          state.section = this.builder.createMarkupSection(normalizeTagName('p'));
+          section = state.section;
+        }
         assert(
           'Markerables can only be appended to markup sections and list item sections',
           section && section.isMarkerable
@@ -271,6 +278,10 @@ class SectionParser {
   }
 
   _updateStateFromElement(element) {
+    if (isCommentNode(element)) {
+      return;
+    }
+
     let { state } = this;
     state.section = this._createSectionFromElement(element);
     state.markups = this._markupsFromElement(element);
@@ -408,8 +419,11 @@ class SectionParser {
   }
 
   _createSectionFromElement(element) {
-    let { builder } = this;
+    if (isCommentNode(element)) {
+      return;
+    }
 
+    let { builder } = this;
     let section;
     let {tagName, sectionType, inferredTagName} =
       this._getSectionDetails(element);
@@ -433,10 +447,9 @@ class SectionParser {
   }
 
   _isSkippable(element) {
-    return isCommentNode(element) ||
-           (element.nodeType === NODE_TYPES.ELEMENT &&
-            contains(SKIPPABLE_ELEMENT_TAG_NAMES,
-                    normalizeTagName(element.tagName)));
+    return element.nodeType === NODE_TYPES.ELEMENT &&
+           contains(SKIPPABLE_ELEMENT_TAG_NAMES,
+                    normalizeTagName(element.tagName));
   }
 }
 

tests/unit/parsers/section-test.js

@@ -615,3 +615,41 @@ test('#parse handles <p> inside <blockquote>', (assert) => {
   assert.equal(sections[1].tagName, 'blockquote');
   assert.equal(sections[1].text.trim(), 'Two');
 });
+
+test('#parse allows top-level Comment nodes to be parsed by parser plugins', (assert) => {
+  let element = buildDOM(`<!--parse me-->`).firstChild;
+  let plugins = [function(element, builder, {addMarkerable}) {
+    if (element.nodeType !== 8 && element.nodeValue !== 'parse me') {
+      return;
+    }
+    let marker = builder.createMarker('oh my');
+    addMarkerable(marker);
+  }];
+
+  parser = new SectionParser(builder, {plugins});
+  let sections = parser.parse(element);
+
+  assert.equal(sections.length, 1);
+  let [section] = sections;
+  assert.equal(section.text, 'oh my', 'parses comment with parser plugin');
+  assert.equal(section.markers.length, 1, 'only 1 marker');
+});
+
+test('#parse allows nested Comment nodes to be parsed by parser plugins', (assert) => {
+  let element = buildDOM(`<p><!--parse me--></p>`).firstChild;
+  let plugins = [function(element, builder, {addMarkerable}) {
+    if (element.nodeType !== 8 && element.nodeValue !== 'parse me') {
+      return;
+    }
+    let marker = builder.createMarker('oh my');
+    addMarkerable(marker);
+  }];
+
+  parser = new SectionParser(builder, {plugins});
+  let sections = parser.parse(element);
+
+  assert.equal(sections.length, 1);
+  let [section] = sections;
+  assert.equal(section.text, 'oh my', 'parses comment with parser plugin');
+  assert.equal(section.markers.length, 1, 'only 1 marker');
+});