Upgrade TinyMCE and switch to CodeMirror plugin

[cahrens]

High Level Description: new version of TinyMCE and change to get rid of the tabbed TinyMCE/CodeMirror editors in favor of a CodeMirror plugin within TinyMCE (triggered from toolbar).

Details:

1. Started with commits in tinymce update to latest version 4.0.16 #2588. Includes deletion of old version of CodeMirror and submission of new version (4.0.16).
2. Includes CodeMirror plugin for TinyMCE (first submission), which has an Apache license.
3. Changes to the HTML edit.coffee file to stop managing switching between CodeMirror and TinyMCE editors (for editing raw HTML and WYSIWYG editing, respectively). Instead, use the CodeMirror plugin.
4. Rewriting of tests, and addition of integration tests for areas that previously weren’t tested.

Unfortunately, I had to edit the code of the CodeMirror plugin and the code for the link and image plugins for the following reasons:

CodeMirror plugin for TinyMCE

1. Needed to change paths of where CodeMirror files are expected to live to match how we store our vendor files. I am currently referencing the un-minified version of CodeMirror, but may change to minified after Removed circuit editor setup code that had always been in codemirror-compressed.js #2959 is merged.
2. Fire events when CodeMirror plugin is shown and saved so that we can swap out static links (in CodeMirror editor only, else user sees broken links appear in TinyMCE Editor behind the dialog).
3. Make sure to strip out the “caret” span that is being used to maintain caret position between TinyMCE and CodeMirror. There is a bug in the plugin that it doesn’t get removed if caret is in style block.

Image and Link plugins

1. Fire an event when the plugin is closed (via OK) so we can rewrite links in TinyMCE view.

Currently no events are fired when a Plugin is triggered or when it goes away. I have submitted a feature request to TinyMCE for this. http://www.tinymce.com/develop/bugtracker_view.php?id=6799

I also posted a comment on the web page for the CodeMirror plugin about the bug related to the caret positional span.

Places to test:

1. HTML module components on unit page
2. Static Pages
3. LMS bulk emailer on the Instructor dashboard
4. Check that annotations usage of TinyMCE is not broken in LMS

New test coverage added (integration):

1. Test for style block being maintained (STUD-1434)
2. Test that all expected toolbar buttons/dropdowns are present.
3. Test for code format toolbar button
4. Test for rewriting of links after using link plugin (test already existed for image plugin).

I have deployed this branch to http://studio.cahrens.m.sandbox.edx.org/

Code reviewers-- you may now start your engines.

1. @marcotuts Can you visually review the CSS (talk to @frrrances about how it was generated)
2. @andy-armstrong and @nasthagiri will do an in-person review with me
3. @dcadams Can you take a look?
4. @mhoeber Will need documentation updates. I did the same bug fix for the link editor as @dcadams had done for image editor.

[ovnicraft]

Hi @cahrens i check the migration guide from tinymce site[1] and the directory name must changed:
tiny_mce -> tinymce, did you check it ?

[1] http://www.tinymce.com/wiki.php/Tutorial:Migration_guide_from_3.x

[cahrens]

@ovnicraft I think the directory name where the vendor source is placed can be anything (as long as path references are correct). I'm not seeing any issues with it still being tiny_mce.

[ovnicraft]

LGTM, i was working with this to set overview course in CMS with tinymce by default, so i will wait to this PR merged.

[cahrens]

@ovnicraft We are interested in having all of the places where raw HTML is exposed use a TinyMCE/Codemirror combo similar to what we use for the HTML xmodule (on unit page). The thought was to have a reusable Backbone View that could be used throughout the product (and would also be used in HTML's edit.coffee file).

[cahrens]

@talbs @frrrances @marcotuts Here is the PR with the TinyMCE upgrade. Sorry to tag you all, but I'm not sure who will pick up the work.

I have restored all the toolbar items we previously had, although I had put them on 2 lines just to make them all visible.

Here is a link to a description of what changed regarding themes:
http://www.tinymce.com/wiki.php/Tutorial:Migration_guide_from_3.x

Note that in addition to the usages in Studio (HTML Editing component on unit page, static tabs), the LMS instructor dashboard uses TinyMCE (via the html xmodule) for bulk e-mail. And there are 2 versions of the instructor dashboard-- the legacy one and the beta one.

To get to instructor dashboard, go to LMS logged in as an instructor in the course (can set "Admin" status in Studio via course team settings). Then go to Instructor tab and look for "email". In this usage, CodeMirror should appear tabbed with TinyMCE as it does in the other locations-- however, currently I am not seeing the TinyMCE editor, even though it is loading correctly.

[ovnicraft]

@cahrens sounds good, BTW i dont know what Codemirror does, i will wokr in some PR in this week in mention you to review. :-)

[cahrens]

@frrrances I did a little debugging of the issue of CodeMirror not appearing for the instructor dashboard bulk email feature. I think the issue is that annotations CSS is setting the z-order for tinymce to something incredibly high, and it is impacting the CSS here as well. Annotations checked in their own version of tinymce (more recent that what we have checked in to master), so I think the problem didn't manifest until the upgrade because the CSS classes are different in the more recent versions of tinymce.

[dcadams]

@cahrens the work I'm doing to replace CodeMirror would require TinyMCE v4 as a pre-req. Do you have a timeframe for this PR being merged?

[cahrens]

@frrrances Should we delete the directory common/static/js/vendor/tiny_mce/skins/studio from the PR?

[frrrances]

@cahrens Yes, that makes sense to me if we're upgrading all instances.

[ovnicraft]

Here is not tinymce,min.js ?

[cahrens]

I don't understand your question, Cristian.

[ovnicraft]

Why you point to tiny_mce.min.js ? is not tinymce.min.js ?

[cahrens]

Got it-- I'll take a look. Thanks.

[ovnicraft]

@cahrens its work here !

[cahrens]

@frrrances Should we get rid of this div (also in LMS template at the end)?

[frrrances]

Since TinyMCE inserts a bunch of stuff after the textarea below, I would like to keep this wrapper div in case we want to contain all of it. Thanks for asking though!

[cahrens]

@waheedahmed This PR deletes the code you just added in #2967. However, I think it should be fine because we are no longer doing the tabbed HTML/Code editor approach. Can you try it out and check?

Note that to bring up the HTML editor, you now press the last toolbar button ("Source code").

[marcotuts]

this looks great from a CSS and interaction standpoint. nice work!

👍

[cahrens]

@andy-armstrong @nasthagiri
Here is the diff for instructor_dashboard.html. The diff for instructor_dashboard_2.html is the same.

And here is the diff for lms/templates/widgets/html-edit.html:

[cahrens]

@lduarte1991 This is the PR to upgrade TinyMCE. I did some smoke testing to make sure that this new version doesn't somehow break your different version of TinyMCE (within the LMS), but you might also want to do some testing.

I have deployed the branch to http://studio.cahrens.m.sandbox.edx.org/. The username/password to access the site is the same as what we use for staging (send me an e-mail if you don't remember it).

[dcadams]

A couple of issues:

I believe, in the image plugin popup, the Image Description field needs to have id and name of "alt" for accessibility.

After adding an image with url /static/.... the code plugin no longer pops up a window and I get a 404 to the static image in the console.

[cahrens]

@dcadams The plugin popup is being rendered by TinyMCE's Plugin Manager. So there probably are accessibility issues, but it's not something we have much control over. Unless there is some sort of accessibility patch for TinyMCE. @davestgermain, can you do an accessibility pass on this version of TinyMCE?

As far as the code plugin not opening, I have noticed that sometimes the plugin window does not reopen until I click in the TinyMCE Editor and then click again on the toolbar icon. I don't think it is related to what is in the Editor. Can you try the workaround and see if it causes the editor to reopen for you?

The 404 is expected, as the image really doesn't exist with the /static/ path. We got those 404 errors before as well (in master). For example,

GET https://studio.edge.edx.org/static/grinning.jpg 404 (Not Found) tiny_mce.js:1
d.create.setContent tiny_mce.js:1
u.mceInsertContent tiny_mce.js:1
r tiny_mce.js:1
k.create.execCommand

Thanks for trying it out! Let me know if you see other things.

[lduarte1991]

@cahrens As of now the version on the live site and the one you deployed don't have Annotator running in the studio so I can't confirm whether it works or not but we can tackle it in our end after you've deployed it. It shouldn't interfere with the LMS version because we are using our own version of tinymce at the moment.

[cahrens]

@lduarte1991 Yes, I was talking about testing the LMS version, not the CMS. I just wanted to make sure there were no conflicts over CSS or global variables between your version of TinyMCE and ours. This upgrade changes the version we use for both LMS and CMS (though I think the only LMS usage is in the Instructor dashboard).

[lduarte1991]

@cahrens Just tested. No issues that I could tell.

[dcadams]

@cahrens After adding an image the image icon remains highlighted and it seems that all of the plugins become non-operational; although the drop-downs still drop down. I tried clicking everywhere but it doesn't change the state of the thing.

[cahrens]

@dcadams I sent you some comments in HipChat. The image icon will be highlighted as long as the image is selected in the TinyMCE Editor (like how the link icon works). But I can't reproduce the plugins not becoming operational. Can you work out the reproduction steps (does it happen for you every time)? Is this on my sandbox, or did you pull the branch?

Any debugging you can do of what is happening would be very helpful, as I can't reproduce the issue myself.

[andy-armstrong]

👍 Looks great, and the tests are fantastic. Thanks for addressing our in person review comments.

My only additional comments are regarding the UX of the CodeMirror modal. @frrrances, I know you tweaked it some and so these may not be possible, but I figure I'll write down my thoughts just in case:

• the keyboard shortcut links are very ugly at the bottom of the modal. Are they necessary, and if not can we remove them?
• do we know if the keyboard shortcuts show differently on a non-Mac machine which doesn't have the command key?
• it would be good if we can pick icons for code block and edit html that are more distinct
• is there any way to get the modal to look more like the new edit modal, i.e. gray border, cancel button showing just as a link without a border, make the buttons a little wider etc.
• Note: I prefer the modal buttons on the right, so would it be possible to do that for the edit modal too?

[cahrens]

@nasthagiri @andy-armstrong @marcotuts @dcadams @mhoeber

OK, I have responded to a bunch of feedback and updated the sandbox (http://studio.cahrens.m.sandbox.edx.org/). Please look over the additional commits and try it out!

These are the changes of note:

1. Fixed FireFox bug with image plugin.
2. Enabled the "advanced" options in the image plugin (border).
3. When an existing image is selected and the image editor brought up, convert the full /c4x links back to /static.
4. Likewise, when an existing link is selected and the link editor brought up, convert full /c4x links back to /static.
5. Don't display the "find" keyboard shortcuts in the CodeMirror plugin. They are ugly, and the functionality only partly works.
6. Fixed visual bug that Lou noted.

[andy-armstrong]

👍 Looks great, thanks.

[cahrens]

Frances is considering changing the icons for "Code block" and "Edit HTML". Otherwise, I think this PR is ready to go. If you disagree, let me know!

[cahrens]

@nasthagiri @andy-armstrong @marcotuts @dcadams @mhoeber

Icons have been updated, and there are some small styling changes. As far as I know, there are no outstanding issues.

[nasthagiri]

The "Insert/Edit Image" icon asks for "Source". What can this "Source" be?
It works when I enter a URL. But it doesn't seem to work when I enter a local File path.
On Staging, the old UI for "Insert/Edit Image" used to explicitly say "Image URL".

So can we rename "Source" back to "Image URL" or something that is more informative? Otherwise, it was unclear to me what was exactly required. And I was surprised when it didn't work with my local file.

[mhoeber]

One minor comment -- in new insert image dialog, advanced tab: the style field is editable, but gets populated with other settings (border, etc). I would make Style field read only.

[cahrens]

@nasthagiri I fixed the typos (your comments did not disappear because they are on a particular commit).

@nasthagiri and @mhoeber I would like to minimize the changes we make to the 3rd party plugins. We had to make a lot to the code editor plugin, but they were unavoidable. For the image plugin, I don't really want to start changing the code and styles. Filing enhancement requests to TinyMCE (this is one of their own plugins, I believe) seems like the way to go.

But I will run this by Lou and @frrrances as well.

[nasthagiri]

👍 Pending the resolution around the wording of "Source" in the image plugin. From my perspective, it would be clearer to users (especially non-HTML users) if it explicitly said "Image URL" (like it did before).

[andy-armstrong]

👍 Thanks for all the attention to detail @cahrens and @frrrances. I really like the way this came out. Changing the button label to be "HTML" instead of an icon resolves all my concerns about the discoverability of the feature while also reducing the confusion with the code block button.

[cahrens]

@ovnicraft As an FYI, @dcadams will be working on hooking up TinyMCE to the places in Studio where we currently show the "raw HTML Editor". I know you also expressed an interest in this, and I'd like to avoid duplicated work.

[mhoeber]

@dcadams please tag me on any JIRA issues and PRs related to this.

[ovnicraft]

Thanks to share it @cahrens so i will wait to comments from @dcadams

[jinpa]

FYI - @dcadams is out of town and offline this week.

On Mon, Mar 24, 2014 at 11:40 AM, Cristian Salamea <notifications@github.com

wrote:

Thanks to share it @cahrens https://github.com/cahrens so i will wait
to comments from @dcadams https://github.com/dcadams

Reply to this email directly or view it on GitHubhttps://github.com/edx/edx-platform/pull/2657#issuecomment-38483942
.

[nasthagiri]

Here is the change log for TinyMCE. We'll give the latest version (4.0.20) a try.
http://www.tinymce.com/develop/changelog/?type=tinymce