Joomla 5 is using tinymce v6 and there are a few breaking changes that need to be reviewed and addressed.
https://www.tiny.cloud/docs/tinymce/6/6.0-release-notes-core-changes/
For example the dropdown formatselect is now blocks so the presets need to be updated accordingly. Also need to consider what happens with existing saved toolbars as without action they will be missing the dropdown on upgrade.
Labels |
Added:
No Code Attached Yet
|
Would you be so nice and create a pr @dgrammatiko
@HLeithner probably @richard67 is the right person, as the breaking parts either have to be patched in the update sql files or in a fn in the admin script.php
I mean the missing part from the pr you mentioned?
thanks, is more needed for conversation between j4 and j5?
Yes, according to https://www.tiny.cloud/docs/tinymce/6/migration-from-5x/#formatting-user-interface-name-changes there are more runtime replacements needed. Actually someone should revisit their docs and check what's missing
FWIW the runtime changes (the str_replace
) is not ideal. The changes should be applied to the DB, so @richard67 maybe something for you?
FWIW the runtime changes (the
str_replace
) is not ideal. The changes should be applied to the DB, so @richard67 maybe something for you?
@dgrammatiko I would do that only if it is absolutely necessary. Last time we have done such stuff was not good, see the fix with #40535 . The better way is to use the right defaults when reading parameters which have not been saved yet, see e.g. #40544 . I can have a closer look on weekend if you specify here what is needed, or contact me on Mattermost, but not today.
the thing you mentioned @richard67 was a simple str_replace on a json object what shouldn't be done ;-)
but I would suggest a php script for migartion if it's more complex or you sql json functions for the manupulation.
But keeping the b/c code would be for ever, since you have to save the tinymce plugin to have correct values right?
FWIW there should be a function on the script.php where the json of the params for extensions could:
There's a repeating pattern, when updating some of the params need some tweaking and would be nice to have a predictable way to doing it without cryptic SQL, just plain easy PHP loops...
My 2c
Good idea. It could be something like the updateassets thing where we have a hardcoded list for the core and extension devs who let their installer script derive from our installer script class can override its with a method using their list and then the same code.
Doing it in script.php has another advantage:
When something goes wrong with a database update for some unrelated reason and people use the fix button of the database checker, it will fix only the structure and so not run any update statements in any update SQL, and so the conversion would never happen. But when it is in script.php and called at the end of the fix like we do it with other stuff, it will be executed and all is fine.
so you would like to have something like this https://laravel.com/docs/10.x/migrations
so you would like to have something like this https://laravel.com/docs/10.x/migrations
@HLeithner If you ask me: No, that’s something completely different. We talk about the json modifications of e.g. the params column in the extensions table.
It's about having migration scripts in php and not in sql files, that's the basic point, laravel also includes the complete abstraction package for db engines and rollback and so on. But the basic thing would be to use php instead of sql files for migration.
It's about having migration scripts in php and not in sql files, that's the basic point
Laravel is doing it right!
Maybe before we reach the migration state of Laravel a simple function for the params is a good idea to test the waters. On the other hand a way to upgrade/downgrade the db from PHP would be awesome
On the other hand a way to upgrade/downgrade the db from PHP would be awesome
And unrealistic ;-) when you break the db upgrade why should the downgrade work then^^ and if it is for up and downgrade between joomla versions I'm pretty sure we don't have the resources for this. But yes I like the idea of downgrade ;-)
There should be another PR that fixes the existing params of the plugin when upgrading and revert the runtime patches
@dgrammatiko and other readers: Question is if we shall add it to the postflight method here
https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L897-L898
so it's only executed when updating from 4.4 to 5.x,
or if we add it to the update method here
https://github.com/joomla/joomla-cms/blob/5.0-dev/administrator/components/com_admin/script.php#L80
so it's executed on every update, also from 5.x to 5.y, just for the case it might have failed when having updated from 4.4 once.
I think the postflight method would be better.
Other opinions?
No strong opinions here: post flight should be fine (I guess this is not an abstract version that would be possible to modify any extension params, if it is then in the regular update cycle would be more helpful)
No strong opinions here: post flight should be fine (I guess this is not an abstract version that would be possible to modify any extension params, if it is then in the regular update cycle would be more helpful)
@dgrammatiko The TinyMCE configuration migration is a very specific thing to that plugin because it's very tied on the configuration structure. You can have an arbitrary number of tool bars and menus and sets and so on. An abstract thing which covers that would be too complicated, and we update the editors to new majors only when we have a new major, that's why I tend to handle it with the postflight. I agree that it could be useful to have a more general method for easier migrations and that could happen with minor updates, too, so that would be called in the update method.
Actually my idea above #40622 (comment) will be handy for couple more extensions for v5: the templates. So, if we assume that 5.3 will be available before the v5 RC (@HLeithner someone really needs to ping the Bootstrap team for their timeline) then both the templates (and all their related styles, and their children styles) need some adjustments to incorporate the -rgb
CSS Variables (in 5.2 I did some voodoo but the usage was not extensive). You could check the Muta template to get an idea of the changes needed. Anyways, specific code that handles specific extension is also fine, just a heads up that tinyMCE won't be the only extension (also apart from the templates and the BS5.3 there's Codemirror that needs a major upgrade, which might also need some tweaks of the params)
@dgrammatiko I agree with your previous comment. As said, the TinyMCE plugin and maybe the Codemirror might be a special case, while the template stuff could be handled in a more general way since the structure of the params cannot be modified by the user, while for the TinyMCE you can have an individual configuration with a different number of sets and toolbars and menus.
Am working on the migration on update from 4.4 to 5.x in script.php. Not sure yet if I will have it ready this weekend.
Nice
So there are 3 things here:
// Version 6 unload removed plugins
$plugins = array_filter($plugins, function ($plugin) {
return !in_array($plugin, ['hr', 'paste', 'print']);
});
$toolbar = str_replace(['fontselect', 'fontsizeselect', 'formatselect', 'styleselect'], ['fontfamily', 'fontsize', 'blocks', 'styles'], $toolbar);
$menubar = str_replace(['fontformats', 'fontsizes', 'blockformats', 'formats'], ['fontfamily', 'fontsize', 'blocks', 'styles'], $menubar);
@dgrammatiko Thanks ... the renaming arrays I had already found, the plugins I might have missed.
If we remove the template plugin then we have to fork it or replace it with our own version, the advanced template is a payed only plugin iirc
we have to fork it or replace it with our own version
Yup
Labels |
Added:
bug
|
Labels |
Added:
Release Blocker
|
@brianteeman Can we consider this issue being fixed since PR #40626 has been merged, which includes also the migration of the parameters on update? Or is there something remaining to be done?
P.S.: And if something remaining, is it still a release blocker?
@brianteeman @richard67 there's a PR for the Brocken source view: #40679
@brianteeman @richard67 there's a PR for the Brocken source view: #40679
@dgrammatiko Yes, I just saw. Thanks for the PR. Anything else remaining to be fixed?
I've asked Harald if he wants to expose the code mirror options to the TinyMCE Sets (ie https://github.com/joomla/joomla-cms/pull/40679/files#diff-6bdf64a4a47bdcc37efb60d1257a5ade08b98727ae6978661ac36e8a50de1ccbR12-R31) If so, some fields have to be introduced in the tinybuilder and also your code needs to fill the existing params (+ the installation sql files)
Not a release blocker but the following cosmetic issues remain
Seperators
The editor no longer displays a | as a seperator between groups of buttons. That's fine but the mockup editor in the plugin still displays the |
Rounded corners
The editor now has rounded courners. Looks odd as the rest of atum doesnt.
Labels |
Removed:
Release Blocker
|
Labels |
Added:
Release Blocker
|
Labels |
Removed:
Release Blocker
|
@brianteeman for the separators use this in the Atum SCSS:
.tox-toolbar__group:not(:last-of-type) {
border-inline-end: 1px solid var(--dark);
}
For the rounded corners, use:
.tox-tinymce {
border-radius: var(--border-radius);
}
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-09-27 11:33:45 |
Closed_By | ⇒ | brianteeman |
You're right. Also it seems the code committed into 5.0 with #37633 is missing parts of Digital-Peak#21
This issue deserves a Release Blocker tag!