Skip to content

Commit

Permalink
🐛 Fixed parser plugin handling of top-level comment nodes
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kevinansfield committed Mar 10, 2020
1 parent f40c273 commit 96710ce
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 6 deletions.
25 changes: 19 additions & 6 deletions src/js/parsers/section.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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));
}
}

Expand Down
38 changes: 38 additions & 0 deletions tests/unit/parsers/section-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

0 comments on commit 96710ce

Please sign in to comment.