-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade JS deps - tinymce #334
Comments
Turns out TinyMCE 7 uses the GPLv2 license which is not compatible with our BSD licenses.
Options
Personally I'd like to avoid swapping to It's not super obvious if we have enough time to swap to a different editor with high confidence - though I think it's worth a try. My personal plan would be:
I think it would make sense to move the tinymce stuff into its own module immediately regardless of whether we have time to swap or not, because that makes it easier to have a new optional module in a minor. Editors worth consideringMost popular WYSIWYG editors these days are not operating directly in HTML - instead they have an internal representation of content (usually in JSON) that could be output to any result. This allows for easier implementation of complex functionality such as real-time collaborative editing - but it also means it's harder to have an allow-list of HTML elements like we do with TinyMCE. All of the below have the same limitation that to have client-side stripping of disallowed HTML elements while always allowing all configured-to-be-allowed elements there's some amount of work involved on our end. Of these I think TipTap would be my preferred option, for the following reasons:
Editors ruled out as not suitable
|
@emteknetnz @silverstripe/core-team Can you please weigh in on this? tl;dr we can't upgrade to TinyMCE 7 so we either need to stick with TinyMCE 6 for now (and eat the maintenance costs of doing so for the next 4 years) or swap to a different editor. The list of alternative editors I've looked at is by no means exhaustive so if there's one you're aware of that's not on this list that could be suitable please shout it out. |
I don't know how palatable staying on v6 for CMS 6 really is given that it is currently out of community security support https://github.com/tinymce/tinymce?tab=security-ov-file#readme
What are the maintenance costs? Presumably it will just stay on an old version? Are you referring to the cost if someone reports a security vulnerability that we then need to fix ourselves on a fork? At that point we may as well just use HugeRTE, which obviously isn't ideal
Big maybe on that one, it may depend on how much hard-coding there currently is between tinymce and entwine/jquery (hopefully none). We may need to do some level of hard-coding just to get something like TipTap functional. I'd view making the editor swappable as a nice to have. It could also possibly be done later on in a minor. I think it's likely that if we wanted to swap to another editor, then everyone now has to use it.
That seems like it may be the most important feature given that a very large proportion of CMS 6 websites will be getting migrated from CMS 5 websites with existing HTML saved in the database?
I'm not sure how much weight I'd put on an ambitious future state that we may or may not prioritize. Given we're being somewhat forced to migrate to a different html editor at short notice close to a beta, it seems like we should put the most weight on "it does a good enough job with the least amount of effort". That's probably not the best way to make a technology choice, though it's kind of the hand we've been dealt here. |
I use Quill actively for several custom projects and it's well designed but definitely not as powerful as Tiny, I would be surprised if most clients wouldn't lose some sort of feature from TinyMCE they use on a daily basis (table properties, formats or some obtuse plugin). A change to Quill or similar would be fine for new projects but for legacy sites it would be a major ballache from looking back at all the editor setups CMS authors have asked for over the years. I'd say HugeRTE would be the best bet since it's likely to be the path of less resistance assuming they keep some level of compatibility. TipTap looks sexy, eyeing up that Page builder! |
My instinct would be to stick with TinyMCE or HugeRTE for now. Then after CMS 6 is released, work on building another editor as a module with a view to that eventually becoming the default editor. Making a decision on a replacement with such limited time is obviously far from ideal, whereas building something out as a module gives the chance to gain feedback/fixes before making it the default experience for everyone. R.e. TinyMCE 6 vs HugeRTE - looks from the readme like HugeRTE is “forked” from TinyMCE 6, but would require people to find+replace I think the main questions I’d have are around how much maintenance effort do you think maintaining TinyMCE would be?
|
@emteknetnz
Yes, that's what I'm referring to. We have to do that already for CMS 5 support anyway - and have already done it with TinyMCE 4 for CMS 4 support.
Any hardcoding that exists would be moved into the TinyMCE module specifically. I don't think there's very much of it. There's obviously the TinyMCE-specific plugins which are easy to move. There's the code to inject TinyMCE into the dom which again is easy to move. Other than that, Similarly, I don't see that we'd need to hardcode anything into admin for a tiptap module to work.
Definitely a nice to have - except if we're replacing it then having the option to stick with an unsupported TinyMCE 6 will make upgrades easier for a lot of people. I don't think "everyone now has to use it" is a stance we have to nor should take.
Sorry, I should have been more specific about what those notes represent. I was just taking quick notes as I looked - just because something's mentioned for one item doesn't mean it doesn't apply to others necessarily. It's not a comparison so much. For this point specifically, it was that you can dump Quill on top of a DOM node and it'll use the HTML in that node to construct its editor. But we wouldn't want to do that anyway - we'd want to pass it some HTML content via an API like we do for TinyMCE - which ALL of the options support in one way or another.
It's a nice thing about that option - it's not the only thing about that option. @wilrThank you for that context, sounds like Quill isn't really suitable for us. @kinglozzer
That's probably the most sensible option. I think moving the TinyMCE implementation into it's own module will make that process a little easier, because that means anyone who wants to use the new editor will be able to simply exclude TinyMCE from their
If that includes third-party plugins, I don't think that's something we should be asking people to do. There is a huge amount of third-party plugins for TinyMCE and given how new this fork is there's no way of telling if it will take off and those plugins will ever be ported.
snyk only lists 8 vulnerabilities that affected TinyMCE - though as more are found in TinyMCE 7 it looks like the "vulnerable version" will just be
If you mean writing a patch from scratch without referring to the way that TinyMCE patched it - it depends what area of the code is affects and how the vulnerability is exploited. If you mean copying a patch over from their codebase into our fork, then yes sometimes it can be a little unclear what the patch is doing, and if we don't know how to exploit the vulnerability we'll just have to hope that the patch does actually fix it in our fork, i.e. that their codebase hasn't changed too significantly from when we forked it.
For a lot of things, yes. There are already a couple of CVEs against TinyMCE that we've decided simply don't apply because they get caught by server-side sanitisation.
Yes - that was our process for the issues that were discovered with TinyMCE 4 in CMS 4 - we looked at how they were patched in TinyMCE 5 and 6, and ported those fixes to TinyMCE 4. Looking at a fork would be no different a solution. RE HugeRTE vs TinyMCE in generalI think if we're sticking with TinyMCE as an editor, we should stick with it directly rather than swap to an unproven fork. It will represent less upgrade pain for projects (plugins are guaranteed to work with TinyMCE but may not work with HugeRTE without modification, for example) and we know what we're getting into maintenance-wise that way. If a CVE is released for TinyMCE 6, we know we have to evaluate it if we're using TinyMCE 6. What if we're using HugeRTE? Does the CVE apply to that or not? Will it be patched by the maintainers of that fork or not? Do we need to take action or not? |
It does seem like the concensus is leaning towards either continuing to use TinyMCE 6 or moving to HugeRTE for CMS 6 which are likely the most sensible options. |
Having a further think about this, if it's between TinyMCE 6 and HugeRTE, then I'd say go with self maintained TinyMCE 6 since it looks like HugeRTE has no traction and is hard to explain to any stakeholders who query why we choose them. A different fork may well end up being the popular unofficial fork that actually gets contributions. If we maintain our own fork we have the freedom to cherry-pick any community fixes that have gone into other forks. That said, I'm still not sure this is the best choice. By doing this we are consciously taking on tech debt. Pros:
Cons:
|
If we swap to another editor immediately, we should still provide a way to keep using TinyMCE 6 for projects that need it for whatever reason - so this is a bit moot.
This is mitigated if in a minor release (say 6.1.0) we introduce an optional module (which we can include in installer by default for new projects) which uses an alternative editor.
If we tell them we're maintaining a fork where we'll be patching security issues, this is mitigated.
I don't see how that's relevant? That's gonna happen regardless of what we choose, it's just now or later. But in any case whenever we swap to a different editor, we should provide a version of the TinyMCEConfig php class bundled with TinyMCE 6. |
I personally don't mind what we choose - but I think time constraints will end up choosing for us, and that choice will be stick with TinyMCE 6 for 6.0.0 and look at an (optional but included in installer) alternative for 6.1.0 |
Given an unlimited CMS squad developer resource (which we definitely do not have) I think the optimal outcome here is to switch to a new default editor and provide projects with the ability to switch to a commercially unsupported module with tinymce 6 if they do not want to deal with the upgrade pain just now Getting the new editor into 6.0.0 rather than 6.1.0 would solve all the cons I listed above Our major release policy does have some wiggle room to delay CMS 6 by a little bit, it seems like it might be worth exploring that if it means we can get a new editor in as it's obviously a critical part of the CMS |
Is this the wiggle room you're referring to? I think it's worth leaning towards the June end of that for sure - which means an early April beta release at the latest which is when this would have to be done by. We have a meeting coming up with Jackson next week and the release timeline is on the agenda. I don't necessarily think it's worth delaying the release just for this, though. I think there is very little risk to using TinyMCE 6 at launch and providing an optional swap to an alternative in a minor release, giving us plenty of time to look into options and not rush to a decision. With all of that said - do you have a preference for what editor we would eventually swap to, given eventually we will have to do so regardless of if it's now or later? |
Yup, I'm not suggesting we go outside of the defined window, just that we assume we'll be releasing at the tail end of the window
I do not as I have no direct experience with any of the other editors, I can only make some assumptions by having a quick glance at them I'm not exactly inspired with confidence so far
It seems like Quill and TipTap are really the only 2 we'd consider. I'm assuming that quill is the one that comes with more out of the box though is harder to do custom stuff with, while TipTap is the one that comes with less (presumably a lot less), though is easier to do custom stuff with. Given we do things like the insert media modal, and the insert link drop down, we need to do quite a lot of custom stuff. Feels like next step here is to spend a few days on a spike / research for both of those options (possibly also EditorJS as it does look powerful just very different), and then make a recommendation as for next steps? |
I still think the next step should be to split the existing TinyMCE 6 implementation into its own module, but if we can't agree on that then yes additional research or a POC or something along those lines would be the next step I think. I'd still recommend we leave that for after all of the other necessary CMS 6 work has finished, unless someone who isn't on the CMS Squad wants to chip in and investigate the options for us in more depth. |
Yes splitting out TinyMCE 6 before the POCs does make sense as it means that if we only get as far as doing that at least we've retained the option to swap out the editor in a minor |
I do't have experience with other editors, but for the sake of existing paid plugins working in CMS 6 it might be best stick with TinyMCE 6 for now and get stakeholders enough time to consider what they can do if we switch to a different editor later on. A question that haven't been asked yet, or that I possibly missed an answer to, about the licenses — is there anything we can do there? Explicitly call out somewhere that we might include GPLv2 sw in a BSD project, or consider switching to GPL for CMS6 or something along those lines? Is there some room to explore this direction at all? |
There will be a way for people to keep using TinyMCE 6 with the existing PHP and Javascript (with probably some changes to namespace since we want to move it into its own module) regardless of whatever else happens, so that's not going to be a concern. The questions are ultimately:
Both of those questions are ultimately answered by whether it's feasible and sensible to implement an alternative editor in time for 6.0.0 with high confidence that it's a good solution. If yes, then the TinyMCE 6 implementation will probably not be commercially supported, though it will be availalble. Otherwise if we have to add an alternative editor in a minor release, I think we have to keep the TinyMCE 6 solution as a commercially supported option until CMS 7. But either way it will be available. |
We can't include GPLv2 anything in a BSD project, because it's a copy-left license and even TinyMCE say "hey we don't really know how this affects other open source projects that use TinyMCE as their editor of choice" so that's not a can of worms I'd like to open. And swapping Silverstripe CMS to GPL2 has similar problems - depending on how you interpret the license, it might mean every project that uses Silverstripe CMS suddenly has to be open source under a GPL license. |
I've created the following issues for next steps, since these two steps do need to happen regardless of whether a new editor is available for 6.0.0 or not:
If there are additional ACs or things that you think are worth investigating, feel free to comment on those issues. I'm going to close this issue for now - we have some next steps, and a decision on whether a new editor will be included in CMS 6.0.0 will be decided partly by how far we get with those before the beta and partly by what is discovered in the investigation spike card, assuming we get around to that card by then. |
Split from #308
See #334 (comment) below
Acceptance criteria
The text was updated successfully, but these errors were encountered: