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

fix: escape character on paste, escape before splitting line #154

Closed
wants to merge 1 commit into from

Conversation

hellasol
Copy link

No description provided.

Copy link
Contributor

@raineorshine raineorshine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! You have identified that decodeEntities is part of the problem. Unfortunately there is a regression—the user should still be able to paste HTML.

Please see if you can get the desired behavior without breaking paste from raw HTML OR paste from formatted text. A good way to test this is to copy the raw HTML and then copy and paste the bulleted list in the original post.

@@ -237,15 +238,14 @@ export const Editable = connect()(({ focus, itemsRanked, contextChain, showConte
// this reflects the format of the source data more than the actual contents
// text/plain may contain text that ultimately looks like html (contains <li>) and should be parsed as html
const plainText = e.clipboardData.getData('text/plain')
const htmlText = e.clipboardData.getData('text/html')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, removing this breaks the desired behavior of being able to paste from a bulleted list (from a webpage or word processor for example).

@@ -1470,7 +1470,7 @@ export const importText = (itemsRanked, inputText) => {
importCursor.pop()
}
}
}, { decodeEntities: true })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely part of the issue. We may have to manually encode characters like & since we do not want < to be encoded. Maybe htmlparser2 has more options.

@hellasol
Copy link
Author

If you want to support html & should not be allowed when you paste. It has to be escaped.
Proper html would look like this:

<li>A</li>
<li>B</li>
<li>C &amp; D</li>

I have solved the task at hand. If you want me to spend more time on it you have to increase the bounty.

@raineorshine
Copy link
Contributor

Ah, good point! In that case, it should handle the following two cases:

  • A
  • B
  • C & D
<li>A</li>
<li>B</li>
<li>C &amp; D</li>

This PR introduces a regression by breaking case (1) (copying formatted text). The bounty will be granted to a PR that solves the task without introducing any regressions. Thank you!

@raineorshine raineorshine force-pushed the dev branch 2 times, most recently from 73b453a to 6c05e04 Compare November 23, 2019 00:16
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 this pull request may close these issues.

2 participants