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.

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

addSection in parserPlugins obliterate previous section content in certain cases #683

Closed
sdhull opened this issue May 23, 2019 · 2 comments · Fixed by #685
Closed

addSection in parserPlugins obliterate previous section content in certain cases #683

sdhull opened this issue May 23, 2019 · 2 comments · Fixed by #685

Comments

@sdhull
Copy link

sdhull commented May 23, 2019

For a demo see https://jsfiddle.net/1rmndk8w/4/

The way I discovered this was by copying & pasting a github comment with an image. I found that the text immediately preceding the image was gone after pasting.

I believe this is because state.text is not properly tracking everything but I'm not exactly intimately familiar with the code so I'm not certain.

      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 = null;
        } else {
          this._closeCurrentSection();
        }
        this.sections.push(section);
      },

One thing that is interesting is that in this case, this.state.section.text has the text but this.state.text is an empty string. Perhaps this check could be changed to include this.state.section.text?

/cc @kevinansfield

kevinansfield added a commit to kevinansfield/mobiledoc-kit that referenced this issue Jun 3, 2019
…ances

closes bustle#683

- `addSection` which is called by parser plugins was removing the section rather than closing it when it contains text due to a missing conditional
@kevinansfield
Copy link
Collaborator

@sdhull you were spot on with the required change to the conditional, PR opened with the change and a test here #685

The reason this.state.text was blank yet this.state.section.text wasn't is because the <a> element parsing calls createMarker() which migrates the temporary this.state.text into a marker in the section.

kevinansfield added a commit that referenced this issue Jun 3, 2019
…ances (#685)

closes #683

- `addSection` which is called by parser plugins was removing the section rather than closing it when it contains text due to a missing conditional
@sdhull
Copy link
Author

sdhull commented Jun 4, 2019

Thank you @kevinansfield! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants