In the detach method of the WYSIWYG integration code, media turns the content string into a jQuery object and returns the result of its html() method. If you have a script tag in your content, it is added to the jQuery object at index 1 with the wrapping div at index 0. html() only returns the html contents of the first matched element, which means that the script tag at index 1 (and any other items that are added to other indexes of the jQuery object) are lost.

This is obviously less than ideal.

CommentFileSizeAuthor
#4 media-wysiwyg-detach.patch1.34 KBeffulgentsia
#3 0001-Issue-1280758-The-WYSIWYG-integration-in-media-destr.patch1.57 KBjames.elliott
#1 0001-Issue-1280758-The-WYSIWYG-integration-in-media-destr.patch1.61 KBjames.elliott

Comments

james.elliott’s picture

james.elliott commented
Status: Needs work » Needs review
StatusFileSize
new 0001-Issue-1280758-The-WYSIWYG-integration-in-media-destr.patch1.61 KB
james.elliott’s picture

james.elliott commented

Something that should immediately stand out is the match length check. I couldn't seem to figure out how to get the regex to not match " as well as the image tags.

james.elliott’s picture

james.elliott commented
StatusFileSize
new 0001-Issue-1280758-The-WYSIWYG-integration-in-media-destr.patch1.57 KB

ah, and I wasn't remembering to group the regex to match more than one image tag.

New patch should fix all.

effulgentsia’s picture

effulgentsia commented
StatusFileSize
new media-wysiwyg-detach.patch1.34 KB

Minor code style cleanup.

Status: Needs review » Needs work

The last submitted patch, media-wysiwyg-detach.patch, failed testing.

effulgentsia’s picture

effulgentsia commented
Status: Needs work » Fixed

Committed to 1.x and 2.x.

ParisLiakos’s picture

ParisLiakos commented
Status: Fixed » Needs work

Sorry but this patch broke the Generic file display.
It hitted me after i updated to 7.x-1.0-rc2 and went through the changes one by one and thats what breaks it:/

steps to reproduce:

-Go to file types in configuration and then to manage file display.
-Set a display to use Generic file.
-Go to add content and use the wysiwyg button to add a file.
-Choose the display you previously set to display generic file.
-Save the node.
-No filename just a random icon instead of the expected file icon with a link to the filename

becw’s picture

becw commented
Assigned: Unassigned » becw

@rootatwc I can duplicate your bug in the 7.x-2.x branch. The patch from comment #10 of #1451316: Clean up wysiwyg-media.js fixes this issue but needs testing.

ParisLiakos’s picture

ParisLiakos commented
Status: Needs work » Closed (fixed)

patch there fixes it indeed so i am closing this