Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Section parser: MarkupSection creating elements inside <li> creates invalid sections #714

Closed
kevinansfield opened this issue Mar 6, 2020 · 1 comment · Fixed by #716
Closed

Section parser: MarkupSection creating elements inside <li> creates invalid sections #714

kevinansfield opened this issue Mar 6, 2020 · 1 comment · Fixed by #716
Assignees
@kevinansfield
Labels
bug

Comments

@kevinansfield
Copy link
Collaborator

We've run into this when using mobiledoc-kit to migrate html content to mobiledoc content. This (invalid but surprisingly prevalent) html:

<ul>
    <h3>Testing:</h3>
    <li>One</li>
    <li>Two</li>
    <li>Three</li>
</ul>

Results in a sections array such as:

[MarkupSection, ListItem, ListItem, ListItem]

ListItem should only exist inside of a ListSection so the Post model throws errors:

TypeError: Cannot set property post of #<ListItem> which has only a getter
         at LinkedList.adoptItem [as _adoptItem] (/Users/aileen/code/migrate/node_modules/@tryghost/mobiledoc-kit/dist/commonjs/mobiledoc-kit/models/post.js:41:23)
         at LinkedList.adoptItem (/Users/aileen/code/migrate/node_modules/@tryghost/mobiledoc-kit/dist/commonjs/mobiledoc-kit/utils/linked-list.js:34:14)
         at LinkedList.insertBefore (/Users/aileen/code/migrate/node_modules/@tryghost/mobiledoc-kit/dist/commonjs/mobiledoc-kit/utils/linked-list.js:71:12)
         at LinkedList.append (/Users/aileen/code/migrate/node_modules/@tryghost/mobiledoc-kit/dist/commonjs/mobiledoc-kit/utils/linked-list.js:54:12)
         at DOMParser.appendSection (/Users/aileen/code/migrate/node_modules/@tryghost/mobiledoc-kit/dist/commonjs/mobiledoc-kit/parsers/dom.js:149:23)
         at /Users/aileen/code/migrate/node_modules/@tryghost/mobiledoc-kit/dist/commonjs/mobiledoc-kit/parsers/dom.js:133:23
         at Array.forEach (<anonymous>)
         at forEach (/Users/aileen/code/migrate/node_modules/@tryghost/mobiledoc-kit/dist/commonjs/mobiledoc-kit/utils/array-utils.js:53:16)
         at DOMParser.appendSections (/Users/aileen/code/migrate/node_modules/@tryghost/mobiledoc-kit/dist/commonjs/mobiledoc-kit/parsers/dom.js:132:36)
         at /Users/aileen/code/migrate/node_modules/@tryghost/mobiledoc-kit/dist/commonjs/mobiledoc-kit/parsers/dom.js:116:15 }

I propose that the section parser should create a ListItem for unexpected children of a list element if it contains any text content, otherwise it should be ignored. PR for that incoming.
@kevinansfield kevinansfield self-assigned this Mar 6, 2020
@kevinansfield
Copy link
Collaborator Author

Looks like there is a deeper issue here, valid html with a MarkupSection creating element inside an <li> will create invalid sections...

<ul>
    <li>One</li>
    <li><h3>Two</h3></li>
    <li>Three</li>
    <li>Four</li>
</ul>

= [ListSection, MarkupSection, ListItem, ListItem]

@kevinansfield kevinansfield changed the title Section parser: Non-LI children inside lists create invalid sections Section parser: MarkupSection creating elements inside <li> creates invalid sections Mar 6, 2020
kevinansfield added a commit to kevinansfield/mobiledoc-kit that referenced this issue Mar 6, 2020
@kevinansfield
Fixed section parser handling of markup sections inside lists
104a91a 
closes bustle#714

- if the section parser hits a list item and it's current state is not a previous list item or a list section then open a new list section using using the LI's parent list type
- prevents `ListItem` sections being created outside of a `ListSection`
@kevinansfield kevinansfield mentioned this issue Mar 6, 2020
Merged
kevinansfield added a commit to kevinansfield/mobiledoc-kit that referenced this issue Mar 6, 2020
@kevinansfield
Fixed section parser handling of markup sections inside lists
b13bebb 
closes bustle#714

- if the section parser hits a list item and it's current state is not a previous list item or a list section then open a new list section using using the LI's parent list type
- prevents `ListItem` sections being created outside of a `ListSection`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kevinansfield kevinansfield

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixed section parser handling of markup sections inside lists kevinansfield/mobiledoc-kit
1 participant
@kevinansfield