-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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(#6180): remove MS Office html comments #6818
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for decap-www canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my 2c review
note: i'm not part of the decap team.
|
||
import { markdownToSlate, htmlToSlate } from '../'; | ||
import { htmlToSlate, markdownToSlate } from '../'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these changes should probably not be part of this commit/pr
@@ -182,6 +183,7 @@ export function htmlToSlate(html) { | |||
const hast = unified().use(htmlToRehype, { fragment: true }).parse(html); | |||
|
|||
const mdast = unified() | |||
.use(rehypeRemoveComments, { removeConditional: true }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is what conditional comments are:
Conditional comments are a legacy feature that was specific to Internet Explorer. They were no longer used in IE 10.
maybe add a comment here documenting why what they are ?
@kav are you still interested in moving this forward? If yes, please address the comments and solve merge conflicts. Thanks! |
Summary
Pasting from Microsoft office is adding a large amount of cruft in the form of conditional comments. The commit removes comments from html paste. Confirmed code paths outside of the visual editor paste are not using that function so no unexpected html comment removal will occur.
Test plan
Added a test and confirmed functionality. Also reviewed codebase for other usage.
Checklist
Please add a
x
inside each checkbox:A picture of a cute animal (not mandatory but encouraged)
fixes #6180