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

Lone HTML <script> tags are stripped on paste #9659

Open
Jules-Bertholet opened this issue May 10, 2021 · 8 comments
Open

Lone HTML <script> tags are stripped on paste #9659

Jules-Bertholet opened this issue May 10, 2021 · 8 comments
Labels
squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@Jules-Bertholet
Copy link

📝 Provide detailed reproduction steps (if any)

I discovered this problem while debugging isaul32/ckeditor5-math#35. The plugin makes it easy to observe the incorrect behavior:

  1. Open https://jsfiddle.net/isaul32/qktj9h7x/
  2. Attempt to copy and paste the first or second equation, with or without surrounding text. The operation will succeed
  3. Attempt to copy and paste the third or fourth equation, along with surrounding text. The operation will succeed.
  4. Attempt to copy and paste the third or fourth equation, without surrounding text. Nothing will be pasted.

The issue is that when content is pasted, _setupPasteDrop()
 from ClipboardPipeline processes the content with HtmlDataProcessor's toView(), which itself calls _toDom(), which processes the pasted HTML via a DOMParser:

_toDom( data ) {
	const document = this._domParser.parseFromString( data, 'text/html' );
	const fragment = document.createDocumentFragment();
	const nodes = document.body.childNodes;

	while ( nodes.length > 0 ) {
		fragment.appendChild( nodes[ 0 ] );
	}

	return fragment;
}

_toDom() only retains nodes in the body of DOMParser.parseFromString()'s response. However, when DOMParser encounters certain tags, like <script>, that are alone and not surrounded by text, it places these tags in the <head> and not the <body>. Therefore, if plugins (like the aforementioned math plugin) use <script> tags to store view elements, those elements can be stripped on paste.

✔️ Expected result

All HTML tags are retained when content is pasted into CKEditor.

❌ Actual result

Lone <script> tags are stripped upon pasting into CKEditor.

📃 Other details

If you'd like to see this fixed sooner, add a 👍 reaction to this post.
@Jules-Bertholet Jules-Bertholet added the type:bug This issue reports a buggy (incorrect) behavior. label May 10, 2021
@martynawierzbicka martynawierzbicka added the support:2 An issue reported by a commercially licensed client. label May 11, 2021
@Jules-Bertholet Jules-Bertholet mentioned this issue May 11, 2021
Closed
@Jules-Bertholet
Copy link
Author

#9674 addresses this

Jules-Bertholet added a commit to Jules-Bertholet/ckeditor5 that referenced this issue May 11, 2021
@Jules-Bertholet
Fix (engine): Don't strip non-Javascript <script> tags from HTML when…
973b531 
… converting to DOM. Closes ckeditor#9659.
@psmyrek
Copy link
Contributor

psmyrek commented Jun 24, 2021 •
edited
Loading

I was trying to reproduce this issue, but without success.

It all depends on what is copied/pasted:

Case 1: plain text is copied from equation input box → plain text is inserted

_toDom.with.plain.text.mp4

Case 2: HTML content is copied from editor view → HTML content is inserted

_toDom.with.HTML.content.mp4

It seems that it has nothing to do with the HtmlDataProcessor#_toDom() method in CKEditor 5.

The same behavior can be observed in your isaul32/ckeditor5-math#34, where in the first video you copied & pasted plain text, but in the second video you copied & pasted HTML content.

Please let me know if my conclusions are correct.

@Jules-Bertholet
Copy link
Author

Jules-Bertholet commented Jun 24, 2021 •
edited
Loading

@psmyrek The first video in the linked issue isaul32/ckeditor5-math#34 doesn't actually demonstrate this issue (#9659). Yes, copying and pasting plain text copies and pastes plain text, this is normal. As for your case 2, the one that is relevant to reproduce this issue: in your video you have ckeditor5-math configured to use span tags; the issue only becomes apparent when the plugin is configured to use script tags (edit: see https://github.com/isaul32/ckeditor5-math#plugin-options for config instructions).

@Jules-Bertholet
Copy link
Author

@psmyrek Video demonstrating the bug

2021-06-24.15-52-00-00.00.25.000-00.00.58.333.mp4

@psmyrek
Copy link
Contributor

psmyrek commented Jun 25, 2021

Thanks for the clarification.

I tried again, this time I think I followed the same steps as you did: I cloned https://github.com/isaul32/ckeditor5-math.git, installed all dependencies and started the development server on http://localhost:8080 with the yarn start. Also the ckeditor5-math package is configured to use script instead of span as its output type.

However, I am still unable to reproduce this error on my side. The copied equation in my case is always a complete HTML content: <html>\r\n<body>\r\n<!--StartFragment--><script type=\"math/tex\">e=mc^2</script><!--EndFragment-->\r\n</body>\r\n</html>. I wonder why on your side the <html> and <body> tags are missing and where did you get the <meta> tag from, that precedes the <script> tag in the copied equation.

ckeditor5-math.configured.to.use.script.mp4

I've checked this issue in latest versions of Chrome, Firefox, Edge and Brave browsers and it works exactly the same in all of them, as I expected, but I wanted to make sure that the browser doesn't have any effect here.

Could you help me reproduce this issue? Do you know what I did different than you? Did I miss something?

@Jules-Bertholet
Copy link
Author

@psmyrek I notice that you have Windows line endings (\r\n) in the HTML snippet in the debugger... maybe this is an OS-dependent-thing? I am on Fedora 34, with GNOME. Also, can you try dragging and dropping the equation? That should also reproduce the issue

@Jules-Bertholet
Copy link
Author

@psmyrek Yes, this does in fact appear to be a Windows-specific behavior: https://docs.microsoft.com/en-us/windows/win32/dataxchg/html-clipboard-format

@psmyrek
Copy link
Contributor

psmyrek commented Jun 29, 2021

Thanks @Jules-Bertholet for the hints, this could be OS-related issue indeed.

@Reinmar Reinmar added the squad:core Issue to be handled by the Core team. label Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Milestone
No milestone
4 participants
@Reinmar @psmyrek @martynawierzbicka @Jules-Bertholet