Editor: Avoid double-encoding sourcecode shortcode content

[aduth]

Fixes #1426

This pull request seeks to resolve an issue which sometimes causes content of a [sourcecode] shortcode to be double-encoded.

This happens because:

• We were not passing an accurate initial value into the setContent arguments when assigning content to TinyMCE from a post which is loaded or a post which had been edited previously within the same Calypso session
• The textarea content was not filtered to decode entities from the received post content

Testing Instructions:

Repeat testing instructions from #1426 (comment)

Also verify that code appears correct when switching between editing modes, and when refreshing while viewing either editing mode (specifically verifying that HTML content is decoded upon load).

cc @alisterscott @azaozz

[matticbot]

Test live: https://calypso.live/?branch=fix/1426-editor-sourcecode-decode

[alisterscott]

This is great, tested it and it works well 👍

Before

After

[gziolo]

It works nicely in the Visual mode:

It still doesn't work properly when editing only in HTML mode:

Steps to reproduce

1. Copy XML example from https://msdn.microsoft.com/en-us/library/ms762271(v=vs.85).aspx.
2. Open edit post page and go to the HTML mode.
3. Paste XML into [code language="xml"][/code] block.
4. Refresh the page.

It looks like it works if we paste the example XML and switch to Visual mode and then back to HTML and refresh. So we might be missing some event callback in the HTML mode. I'm not really sure :)

I also think that this PR is a really great improvement, so if that it is really hard to fix then we may deploy it as it is. In that case I'm giving 👍 to 🚢 . Code changes look good, but I must admit I haven't worked with with TinyMCE API for couple of years :)

[alisterscott]

I can confirm the issue that @gziolo is seeing above.

It seems to be only happening when you have the HTML editor view open and open the post directly via a URL (refresh or otherwise)
If you publish directly from the HTML view it publishes the code correctly
If you come into the editor via the posts page into the HTML view, it appears better, but double quotes appear as "
Having the Visual editor open works fine.

[aduth]

The issue seems to be an intermittent race condition which occurs if the post contents have loaded from the API before TinyMCE has finished loading, resulting in this condition not passing (and therefore the content not being filtered).

If you come into the editor via the posts page into the HTML view, it appears better, but double quotes appear as "

Only a handful of entities are replaced. This is a holdover from the original forked plugin, so I'm not sure whether there was a purpose to not include more. Lodash's _.unescape or our own lib/formatting/decode-entities might be more reliable here.

[aduth]

I decided to address the issue by backing out previous changes to filter the content using the TinyMCE plugin and instead always decode entities when assigning the textarea content into state. I think this may have simply been a case of React gotchas related to HTML entities, as demonstrated in the following JSBin:

http://jsbin.com/hiquyoxose/edit?html,js,output

@gziolo @alisterscott Would you mind taking another look?

[alisterscott]

This is great, thanks.

I tested all scenarios listed above including editing from posts list, editing directly, refreshing, using both HTML and visual views and couldn't get it to break the code formatting 👍

[gziolo]

It works way much better as you can see below:

I was still able to find some strange edge case, sorry about that :)

I tested also with JavaScript code and works nicely, too :)

Steps to reproduce

1. Open editor in HTML mode.
2. Add code block with XML code.
3. Switch to Visual mode.
4. Click Update button to make sure it is saved (on the screencast I didn't click, that's why it asks to restore).
5. Refresh the page.
6. You should see only text nodes from XML code.

[aduth]

@gziolo It's like playing a game of editor whack-a-mole!

Appears this latest issue was a consequence of previous change decoding entities before setting value of textarea. Looking closer, if TinyMCE is initialized using the value of the textarea (decoded), entities need to be replaced before setting to the visual editor, else they appear as DOM nodes. This is unlike other cases where the editor is initialized by content from the REST API, which is already escaped.

Kinda confusing, so I added a clarifying comment and improved tests for the plugin along with the fix in f8e1220.

[alisterscott]

I did some more testing but couldn't break it; I'll leave that to the master of breaking it: @gziolo

I did notice that switching between HTML and Visual makes the editor think there are changes so it gives warnings about leaving the editor when nothing has actually changed. When saving the 'changes' in this situation a new revision isn't created so there aren't actually changes. Any ideas why @aduth?

[gziolo]

I did notice that switching between HTML and Visual makes the editor think there are changes so it gives warnings about leaving the editor when nothing has actually changed. When saving the 'changes' in this situation a new revision isn't created so there aren't actually changes. Any ideas why @aduth?

@alisterscott I double checked how it work on production, and it seems like it isn't something new. We should open follow up PR, because switching between modes shouldn't trigger any change in the content.

I tested again, and everything works perfectly fine. Great job @aduth! 🎉

[aduth]

I did notice that switching between HTML and Visual makes the editor think there are changes so it gives warnings about leaving the editor when nothing has actually changed. When saving the 'changes' in this situation a new revision isn't created so there aren't actually changes. Any ideas why @aduth?

Yeah, it's a known issue related to differences in how the content is formatted between TinyMCE and the textarea fields. It's tracked at #6869 (though the "double prompt" aspect is since resolved), with an attempted unfinished fix at #6880 by @nylen .