Skip to content

Commit

Permalink
handle nested section-creating elements correctly in SectionParser
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinansfield committed Dec 10, 2018
1 parent 4bcbec6 commit dfcadc3
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 30 deletions.
35 changes: 34 additions & 1 deletion src/js/parsers/section.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,13 @@ class SectionParser {
let isNodeFinished = false;
let env = {
addSection: (section) => {
this._closeCurrentSection();
// avoid creating empty paragraphs due to wrapper elements around
// parser-plugin-handled elements
if (this.state.section.isMarkerable && !this.state.text) {
this.state.section = null;
} else {
this._closeCurrentSection();
}
this.sections.push(section);
},
addMarkerable: (marker) => {
Expand Down Expand Up @@ -148,6 +154,33 @@ class SectionParser {
return;
}

// handle closing the current section and starting a new one if we hit a
// new-section-creating element
if (this.state.section && !this.state.section.isListItem && !isTextNode(node) && node.tagName) {
// handle lists nested inside wrappers
if (this.state.section.isListSection) {
this.parseListItems(node.childNodes);
return;
}

let tagName = normalizeTagName(node.tagName);
if (contains(VALID_MARKUP_SECTION_TAGNAMES, tagName) || contains(VALID_LIST_SECTION_TAGNAMES, tagName)) {
// avoid creating empty paragraphs due to wrappers elements around
// section-creating elements
if (this.state.section.isMarkerable && !this.state.text) {
this.state.section = null;
} else {
this._closeCurrentSection();
}
this._updateStateFromElement(node);

if (this.state.section.isListSection) {
this.parseListItems(node.childNodes);
return;
}
}
}

switch (node.nodeType) {
case NODE_TYPES.TEXT:
this.parseTextNode(node);
Expand Down
42 changes: 13 additions & 29 deletions tests/unit/parsers/section-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,35 +225,6 @@ test("#parse handles multiple headers in list item", assert => {
});

// see https://github.com/bustle/mobiledoc-kit/issues/656
//
// this is a minimal example of markup that was causing errors when copy/pasting
// from Medium. The original markup stucture that was pasted looked something like this:
//
// <section>
// <div>
// <div>
// <h4>title</h4>
// <ul><li>one - one</li></ul>
// <figure><img /></figure>
// <ul><li>two - one</li></ul>
// </div>
// </div>
// </section>
// <section>
// <div><hr /></div>
// <div><div><br /></div></div>
// </section>
//
// NOTE: because DOMParser passes each top-level element to SectionParser rather
// than passing the leaf-most section with content, the SectionParser needs to
// deal with nested wrapper elements (section->div->div->[h4,ul,figure,ul])
//
// the error being thrown was
// ---
// Uncaught TypeError: Cannot read property 'append' of undefined
// at SectionParser._createMarker
// ---
// the line in question was `state.section.markers.append(marker);`
test('#parse handles list following node handled by parserPlugin', (assert) => {
let container = buildDOM(`
<div><img src="https://placehold.it/100x100"><ul><li>LI One</li></ul></div>
Expand Down Expand Up @@ -281,6 +252,19 @@ test('#parse handles list following node handled by parserPlugin', (assert) => {

let listSection = sections[1];
assert.equal(listSection.type, 'list-section');
assert.equal(listSection.items.length, 1, '1 list item');
});

test('#parse avoids empty paragraph around wrapped list', (assert) => {
let container = buildDOM(`
<div><ul><li>One</li></ul></div>
`);

let element = container.firstChild;
parser = new SectionParser(builder);
let sections = parser.parse(element);

assert.equal(sections.length, 1, 'single list section');
});

test('#parse skips STYLE nodes', (assert) => {
Expand Down

0 comments on commit dfcadc3

Please sign in to comment.