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(#6180): remove MS Office html comments #6818

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kav
Copy link

@kav kav commented Jun 24, 2023

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)
1ECF756A-ACE6-4A16-BD1C-B9D121446AB7_1_105_c

fixes #6180

@netlify
Copy link

netlify bot commented Jun 24, 2023

Deploy Preview for decap-www canceled.

Name Link
🔨 Latest commit 4d5a1b2
🔍 Latest deploy log https://app.netlify.com/sites/decap-www/deploys/649637fb0366070007b86e3f

Copy link

@xaiki xaiki left a 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 '../';
Copy link

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 })
Copy link

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 ?

@martinjagodic
Copy link
Member

@kav are you still interested in moving this forward? If yes, please address the comments and solve merge conflicts. Thanks!

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

Successfully merging this pull request may close these issues.

Copy pasting from word document leads to long comments within files
3 participants