Content not properly escaped for use in JSON when inserting media in WYSIWYG

I'm currently using:
Media 7.x-2.0-alpha3+77-dev
File Entity 7.x-2.0-alpha3+17-dev
CKEditor 7.x-1.13+22-dev

Unfortunately I cannot update to latest dev because I am using the patch at #2164823: Fix for replacePlaceholderWithToken in media_wysiwyg.filter.js to fix another bug, so it's not viable to update until #2126755: Improve Media's WYSIWYG Macro handling is committed and then the CKEditor bug mentioned in #97 & #99 of that issue is also fixed.

If you're using TinyMCE and it works then maybe this is a CKEditor problem.

I've had a little look into this and the file is inserted into the WYSIWYG properly with HTML encoded quotes and displays its preview as it should, however after that when you save the page or click the "Switch to plain text editor" link it gets messed up - the HTML encoded text gets decoded into the token, then later when it tries to convert the token back to an image it gets stuck because the quote is not encoded for HTML or JSON.

This seems to be unrelated to input filters and appears to be a media module issue.

It seems to be in media_wysiwyg.filter.js the function:

    /**
     * Replaces the placeholders for html editing with the media tokens to store.
     * @param content
     */
    replacePlaceholderWithToken: function(content) {
      Drupal.media.filter.ensure_tagmap();
      // Convert all xhtml markup to html for reliable matching/replacing.
      content = content.replace(/[\s]\/\>/g, '>');

      // Re-build the macros in case any element has changed in the editor.
      // Wrap the content to be able to properly use replaceWith() and html().
      content =  $('<div>' + content + '</div>');
      $('.media-element', content).each(function(i, element) {
        $(this).replaceWith(function() {
          var markup = Drupal.media.filter.outerHTML($(this));
          var macro = Drupal.media.filter.create_macro($(this));
          // Store the macro => html for more efficient rendering in
          // replaceTokenWithPlaceholder().
          Drupal.settings.tagmap[macro] = markup;
          return macro;
        });
      });
      return content.html();
    },

When it calls $(this).replaceWith() on the media element the replaceWith() function decodes HTML entities.
It should also be noted that the return at the end of content.html() would also decode HTML entities even if replaceWith() didn't.

So then the HTML returned from replacePlaceholderWithToken() has decoded quotes and that's where the problem starts.

If they didn't get decoded when token replacement happens then it would successfully replace back to markup again later.

I should also mention that it isn't so much the jQuery functions that are the problem but the way the browser deals with the HTML.

I had a little play with double HTML encoding things but there were other issues with that so I tried decoding the entities before using JSON.stringify() and that looked more promising.

An example of the kind of thing that might work here is in media_wysiwyg.filter.js in the extract_file_info() function change the line:

            file_info.attributes[a] = value;

to :

            file_info.attributes[a] = String(value).replace(/"/g, '"');

To test add an image with a title that has a double quote in it.
Without this change it works fine on initial insertion of the media but after switching between plain text & wysiwyg modes the media breaks completely (staying as a token).
With this change you see the encoded quote as " instead of " on initial insertion (regression) but then when after switching between plain text & wysiwyg modes everything works as expected.

Because of that little regression this still nees some work.
I also have only tested with ckeditor and only in firefox at this stage so I'm not sure yet what other issues it may cause to do this.

Hopefully I will have a bit of time to investigate further in the next couple of days.

Looks like this has already been fixed in http://cgit.drupalcode.org/media/commit/?id=72be229 from issue #2126755: Improve Media's WYSIWYG Macro handling with basically the same fix as I mentioned in #9.

Closing this as a duplicate.

Reopening as I have had a look at the solution that was committed in that other issue and it still has problems.

It has the problem I mentioned in #9, where upon initial insertion of the image the user can see excaped content like " (for example if there is a quote in the title and a user hovers over the image in the WYSIWYG).

I'm not sure if that is a problem that needs fixing though because once you save the content that problem goes away.

The other problem with the committed solution is that it only works for the first quote in a string. If I have a title with 2 quotes things still break.

Here is a patch for latest dev.

I'm using an equivalent patch and it works however I am a little behind latest dev due to another bug in that version so I haven't actually tested this one with latest dev.

I'll test this one a bit later to be sure.

I do not think that place in the code is the cause of the issue. I don't know why it's there either, the explanation offered by the comment seems completely backwards too. JSON has no issues with ", but it could maybe have issues with ". EDIT: (Heh, even this actual comment field has issues with rendering \\"...)

Yeah, the problem is not " but the fact that " gets converted to " before it gets json encoded (jQuery markup manipulation causes the browser to decode the html entities).

I agree that my solution was not optimal though, my knowledge of the media module is pretty limited and I have yet to have time to put into it, so I'm happy to be schooled on this :)

I will review your patch ASAP, thanks.

With the patch in #14 applied I get lots of errors like this:

Notice: Undefined index: fields in media_wysiwyg_token_to_markup() (line 122 of media/modules/media_wysiwyg/includes/media_wysiwyg.filter.inc).

Agreed, this seems to no longer be an issue.