- First commit to issue fork.
- Merge request !5Issue #3296778: Automated Drupal 10 compatibility fixes โ (Merged) created by lukey
- ๐ฎ๐ณIndia ajeet_kumar gurugaon
I reviewed with patch https://www.drupal.org/files/issues/2023-04-06/3273358-48.patch โ and found Drupal 10 deprecated issues as
..\contrib\ckeditor_templates\src\Form\CKEditorTemplatesDialogForm.php
Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.. After resolving working as expected CKEditor.
- ๐ฎ๐ณIndia ajeet_kumar gurugaon
Drupal10 CKEditor_template CKEditor5 compatibility 3273358-48#51.patch
interdiff-3273358-48#51.txt - Status changed to RTBC
over 1 year ago 10:03am 3 May 2023 - Status changed to Needs review
over 1 year ago 2:28pm 26 May 2023 - ๐ณ๐ฑNetherlands falco010 ๐ณ๐ฑ The Netherlands
@ajeet_kumar Your patch has way more changes than needed.
To have Drupal 10 compatibility (to at least upgrade to it), you can use the existing fork that Lukey created above:
https://git.drupalcode.org/project/ckeditor_templates/-/merge_requests/5The fork can be used by adding this to your composer.json repositories:
{ "type": "package", "package": { "name": "drupal/ckeditor_templates", "version": "1.x-dev", "type": "drupal-module", "source": { "type": "git", "url": "https://git.drupalcode.org/issue/ckeditor_templates-3296778.git", "reference": "567b63b96949f625ed6aa00af9385e1e59c971f5" } } },
And run the following command:
composer require drupal/ckeditor_templates
- ๐ฌ๐งUnited Kingdom Tom M Fallon
Tom M Fallon โ made their first commit to this issueโs fork.
- Status changed to RTBC
over 1 year ago 5:27am 17 July 2023 - ๐ฆ๐บAustralia gordon Melbourne
I have been doing a lot of testing of this as a part of moving to Drupal 10. I have not found any issues and would make it so much easier.
If you can commit this would be such a huge help for my client.
- ๐ฉ๐ชGermany vincent.hoehn Dresden, Germany
Hello folks. We used this module and needed the functionality urgently for CKEditor5 and Drupal 10, so we developed a plugin and module. For all of you who use the functionality, check out ckeditor5_template โ , maybe it's helpful for someone!
- ๐ฆ๐บAustralia gordon Melbourne
I did some more checking and I have removed the composer.json as it added no benefit and a better version will be generated by drupal.org
- ๐ฌ๐งUnited Kingdom Tom M Fallon
Hey @Gordon
I'm a little bit confused by this. At the moment most contrib modules I've ever come across contain a composer.json.
The reason for adding it was that I was getting
"No valid composer.json was found in any branch or tag of https://git.drupalcode.org/issue/ckeditor_templates-3296778.git, could not load a package from it. "
This error has now returned as my commit was reversed.
The benefit of having a composer.json (even if its replaced later via drupal) is it makes it a lot easier to interact with an issue queue branch. As it stands I've really only been left with the choice to fork this project until this issue merges.
- ๐จ๐ฆCanada smulvih2 Canada ๐
Agree with @tom, we should include a composer.json file in the module.
Can we get a D10 version released ASAP? I created a custom repository for this module (similar to comment #10) in order to bring this module up to D10. I also have ckeditor_templates_ui module in my build, which does have a D10 version. The only issue is that ckeditor_templates_ui requires ckeditor_templates, so in order to get working with D10 I would have to create a MR for ckeditor_templates_ui that points to the MR for ckeditor_templates, which I don't want to to. ckeditor_templates_ui is mandatory for my project in order to manage the templates.
- ๐ฆ๐บAustralia gordon Melbourne
Yes can we please get this committed. It would make things much easier.
However I am using another tool called Drupal lenient, which can install modules that need to be that donโt quite match the criteria.
I am currently down to 2 remain modules which donโt have A d10 release.
- ๐จ๐ฆCanada b_sharpe
Just adding a simple D10 ready (no CKE5) to use with composer.
- ๐ฎ๐ณIndia jaspreet21
I am adding the patch which support Drupal coding standards as well in addition to comment number 18.
- ๐ฆ๐บAustralia gordon Melbourne
I merged in the @jaspreet21 changes. However I didn't include the changes to the info.yml as retaining compatibility with Drupal 8 is really not realistic.
- First commit to issue fork.
- ๐ฉ๐ชGermany marcoliver Neuss, NRW, Germany
I agree with Tom that the composer.json should not have been deleted, because not composer refuses to load the module from the issue fork. I've reverted the commit that deleted the composer.json.
- ๐ง๐ฉBangladesh captain_haddock
Updating patch with changes until 86558271
- ๐ฆ๐บAustralia dpi Perth, Australia
Restoring to prevent ๐ Automated Drupal 10 compatibility fixes Needs review
- ๐บ๐ธUnited States mtift Minnesota, USA
Code looks good, and I can confirm that the patch works. With Upgrade Status enabled and the patch applied, I see this:
- ๐ฉ๐ชGermany Istari
Can also confirm that patch with Upgrade Status enabled works.
- ๐บ๐ธUnited States phernand42
After updating to Drupal 10.1.5 I started getting the following error in node edit screen even after applying patch in #23.
Deprecated function: Creation of dynamic property Drupal\ckeditor_templates\Plugin\CKEditorPlugin\CkeditorTemplates::$themeExtensionList is deprecated in Drupal\ckeditor_templates\Plugin\CKEditorPlugin\CkeditorTemplates->__construct() (line 58 of modules/contrib/ckeditor_templates/src/Plugin/CKEditorPlugin/CkeditorTemplates.php).
I found that declaring the $themeExtensionList property at the class level, before it's used in the constructor made the error go away.
/** * Drupal Theme Extension List. * * @var \Drupal\Core\Extension\ThemeExtensionList */ private $themeExtensionList;
I'll revise patch in #23 to include this just in case anyone else encounters this error.
- ๐บ๐ธUnited States phernand42
Patch re-rolled to fix error mentioned in #27
- First commit to issue fork.
- ๐ฆ๐บAustralia pasan.gamage
The dependency for CK4 should be updated too I think.
https://git.drupalcode.org/project/ckeditor_templates/-/merge_requests/5... - ๐ซ๐ทFrance steveoriol Grenoble ๐ซ๐ท
For D10, dependency should be changed in the "ckeditor_templates.info.yml" file from
dependencies: - drupal:ckeditor
to
dependencies: - drupal:ckeditor5
- First commit to issue fork.
Updated the dependency to the ckeditor5 module as suggested in previous posts.
- ๐ซ๐ทFrance devil2005
Hi,
Can you release a stable version please ?
I have to use zip file and repository isn't up to date (branch 2.x is 3 years old)Thanks
- First commit to issue fork.
-
lucaslg โ
committed 831ea848 on 8.x-1.x authored by
Lukey โ
Issue #3296778 by gordon, Lukey, phernand42, Tom M Fallon, marcoliver,...
-
lucaslg โ
committed 831ea848 on 8.x-1.x authored by
Lukey โ
- ๐ซ๐ทFrance lucaslg
Hi,
Sorry for the delay, I know people have been waiting for it. I didn't have much time to work on the subject.
But I did try to follow along the conversation. At first, I didn't understand why people worked on this version as CKEditor 4 support stopped at the same time as Drupal 9. But I discovered in the conversation that having a D10/CK4 version ease the transition of some project to Drupal 10.
I am not sure if the ThemeExtensionList service is the good one to use as it is marked @internal in the documentation and therefore should not be used as a public api if I read it correctly. But that can be corrected later in another issue if needed.
Thanks everyone who participated.
I'll look at the CKEditor 5 issue very soon to have another release.
Sorry again for the delay. - Status changed to Fixed
12 months ago 10:11pm 30 November 2023 - ๐จ๐ฆCanada joelpittet Vancouver
@lucaslg Thank you for tackling this! On big piece of the puzzle is in!
Automatically closed - issue fixed for 2 weeks with no activity.