Update CKEditor to 4.18.0

[daguiler]

Summary

Updates CKEditor to v4.18.0

Notes

• The copyright wording has changed, affecting pretty much all files.
• To make reviewing this PR easier, I updated the code but not the copyright headers in the first commit. So you can take a look there to see functional changes. Everything's minified, so it's hard anyway.
• The WebSpellChecker plugin for v4 (wsc) has reached EOL and is no longer shipped with CKEditor.
• Same with the Flash plugin.
• The moono skin replaced all the individual images with one single bitmap, so you will see a lot of deleted images in the moono folder.

[mitchelsellers]

Historically we had an upgrade issue with the last upgrade, where toolbar items and other plugins moving resulted in things breaking & not loading. I believe that this will introduce a similar issue.

Additionally, there is nothing in here to remove the older 4.15.1 code, should we consider options to do this cleanup? As installations that have upgraded recently will now have 3 versions installed?

@dnnsoftware/approvers ?

[mitchelsellers]

Additionally, if CKEditor isn't supporting something and not shipping it, I do not believe we should be shipping it any further. Which means that we wouldn't ship the Flash & WebSpellCheck stuff.

[daguiler]

there is nothing in here to remove the older 4.15.1 code

Good point about clean up. During my tests, I uninstalled the CKEditor provider using the Extensions panel, with the delete files option checked, and installed a new version, numerous times. This works well. Removing the extension effectively removes all associated files. But I'm not sure what happens during upgrade. Aren't extensions removed and reinstalled? I'll give it some test tomorrow.

[mitchelsellers]

@daguiler In an upgrade, without a cleanup task, nothing is done to the existing files at all. In the case of CKEditor, it is very possible that additional files are present in the older directories, for example if the users added a custom toolbar item. In those situations we don't necessarily want to delete the directory/file.

This might be a larger conversation.

@valadas Do you remember the upgrade issues that we spent all of that time troubleshooting? Just trying to avoid doing that again ;)

[daguiler]

I do not believe we should be shipping it any further

Yes. These two plugins were not included in this bundle.

[valadas]

As for not destroying existing plugins, we now have support for globbing patterns in the installer code, maybe this could be one way to skip the plugins folder https://docs.dnncommunity.org/content/tutorials/extensions/dnn-manifest-schema/index.html#components I know that's what I do with some of my modules for good control over what to delete and what to keep. I made that PR in v9.5.0 #3516

Components can be ordered in the manifest, so I am thinking we do a glob that would clean the folder leaving the plugins in place before the resources are extracted there...

[daguiler]

I tested the most basic upgrade scenario, from latest 9.10.2 to this pull request version.
It worked fine, but as Mitch said, it left the 4.15.1 folder behind and now I see both 4.15.1 and 4.18.0.
All paths point to 4.18.0 though.

[daguiler]

Second test:

1. Installed 9.10.2.
2. Created a page, added some content
3. Added a new group to the "Full" CKEditor toolbar, including SCAYT and Flash. At this point I noticed that the "WebSpellChecker" plugin is no longer working (as expected). It tried to invoke some remote service and got 403 responses.
4. Upgraded to this version
5. Opened the page in edit mode
6. Verified that the toolbar change was preserved, except the Flash button was no longer there (as expected). The SCAYT plugin still working fine.

[daguiler]

After manually deleting the 4.15.1 folder, still working fine

[daguiler]

I added 2 commits to address the cleanup part.

I'd love some real world experience here, but my assumptions are:

1. Settings like toolbar customizations are not stored as files but as records in the database, so they are preserved after the upgrade

2. DNN doesn't provide a built-in way to add or remove CKEditor plugins, so we should be able to replace the js/ckeditor/version folder entirely when we decide to ship a newer version of the editor. Keeping the editor up to date is important as new vulnerabilities are discovered. We could highlight this in the release notes for users that installed additional plugins, that they will have to manually re-install them after the upgrade.

@valadas One note about the cleanup "glob": it works fine and it removes all of the files, but it leaves the empty folder structure behind. In this case it leaves 124 empty folders under js/ckeditor/4.15.1. I tried to find ways to specify the 4.15.1 folder itself to be deleted, but I don't think it's possible.

[daguiler]

I noticed this paragraph in the docs:

but I'm not sure which methods it is referring to.

[valadas]

but I'm not sure which methods it is referring to

🤔 I remember there was some issue with globbing folders and I believe a folder needs to be empty in order to be able to delete it. Maybe we can make some installer adjustments to optionally allow deleting a folder even if it is not empty... I am not sure, and it is a pain to test.

[valadas]

Also, I added the noteworthy label to this so to not forget to call out attention to testers for this. I do not use custom plugins and have a feeling few of us do that are aware of this PR 😄

[valadas]

Awesome!