Drupal 10 & CKEditor 5 readiness

Thank you @sker!

We also miss an upgrade path. For now, if I switch an existing CKE to CKE5 in Drupal UI, I get:

The CKEditor 4 button DrupalFile does not have a known upgrade path. If it allowed editing markup, then you can do so now through the Source Editing functionality.

The button is removed and the plugin config (which extensions are allowed) is reset.

Also, I get both the "edit link" and the "edit file" buttons on both usual links and file links.

@GaëlG

We also miss an upgrade path. For now, if I switch an existing CKE to CKE5 in Drupal UI, I get:

I totally forgot about that, I'll take a look when I have a chance.

Also, I get both the "edit link" and the "edit file" buttons on both usual links and file links.

This is the same behavior when you right click on a link on the ckeditor 4 version. I tried to create an unique popup view for uploaded file links but I had to pulled in a lot of duplicated code from the LinkUi plugin, so I decided to just revert the changes and reuse the popup from LinkUI.
See this https://git.drupalcode.org/project/editor_file/-/merge_requests/3/diffs?commit_id=b6f34abe012a79cfab5694104ad2b2ac32797ca7 if you believe it would better to use a dedicated popup for file links.

This is the same behavior when you right click on a link on the ckeditor 4 version

Oh, you're right, I never noticed. Yes, a dedicated popup would be better, but in the meanwhile, if it's easier to do, I'm wondering if no popup at all for file links wouldn't be better. After all, it's quite unusual to change the file of a file link.
I don't have much time for it in the coming weeks, but maybe using another model name than "linkHref" would do the trick. See https://git.drupalcode.org/project/editor_file/-/merge_requests/2#note_1...
Anyway, as it's already in CKE4, this could be a follow-up issue.

As I check the drupalfile plugin is saving configuration data in the third-party settings of the editor. So here we can't use the standard method (mapCKEditor4SettingsToCKEditor5Configuration()) to migrate the configuration of the plugin from CKEditor 4 to 5. To migrate the third-party settings from Ckeditor 4 to 5, I created a method migratePluginThirdPartyConfiguration() in the CKeditor5Plugin/File.php, the third-party settings will fetch from the ckeditor4 editor and saved to the ckeditor5 editor (https://git.drupalcode.org/project/editor_file/-/merge_requests/3/diffs?...). I have not found any other standard way to migrate the third-party settings.

I created a new MR that uses plugin configuration instead of third party settings. I kept getting errors because the config was stored in third party settings due to checks in \Drupal\ckeditor5\Plugin\Validation\Constraint\EnabledConfigurablePluginsConstraintValidator::validate. I'd also think switching to CKEditor5 would be a good point to store everything correctly :)

Updated the test according to https://www.drupal.org/project/editor_file/issues/3232053#comment-15084068 🌱 Drupal 10 & CKEditor 5 readiness Needs review .

Added a few suggestions to MR 4

Thanks for updates! it looks fine to me.
I will leave the ticket as "Needs review" so maintainer can review that too.

sker101 → closed merge request !3

I just closed the MR #3 for less confusion as the MR #4 should have all the changes from MR #3.
I believe people should be using/making new commits to MR #4.

I have manually tested MR #4 with drupal/core 10.1.2, everything seems to work fine. The patch applies, there are now errors when you switch the editor from ckeditor to ckeditor5 and I can still upload files after switching.

This issue appears to have been thoroughly tested and according to the comments on MR #4, any remaining feedback is being applied. I think we can now mark this as "RTBC".

Attached is a patch file with the current status of MR #4. We prefer patch files in our projects because we use composer patches and drupal.org merge request URLs are considered dangerous; see https://github.com/cweagans/composer-patches/issues/347.

@gaëlg Why remove the bug fix from b10f1269e4dcda05c25eee0abcc0330781c523f1 in the latest commits to Merge request #4? Must this bug be coming from somewhere else than this module?

@percoction: I guess this bug will not happen anymore as:
- LinkUI is now required: https://git.drupalcode.org/project/editor_file/-/merge_requests/4/diffs?...
- we use LinkUI after it's been initialized: https://git.drupalcode.org/project/editor_file/-/merge_requests/4/diffs?...

I'm updating a website I'm unfamiliar with to Drupal 10 / CKE5 at the moment. I had to update this plugin and applied the patch from #39.

There seems to be something weird going on on the text format admin screen, but I was finally able to save it when adding a filetype eg: "pdf" under: CKEditor 5 plugin settings > File

However actually loading a WYSIWYG text field results in a JS error:
ckeditor5.js?rztqq8:472 TypeError: Cannot read properties of null (reading 'once')

.. disabling the plugin buttons will make the WYSIWYG work again. The WYSIWYG is displayed in two different themes: "Seven" and a Bootstrap based custom theme. It happens in both.

Advice or hints greatly appreciated. :-)

Just saw it too. Interestingly if I save the /admin/config/content/formats/manage/basic_html and enable the plugin, then it works. If import from config, the above error happens. The configs just have the new editor_file config in it.

From the code part, I don't think this is correct:

editor_file/js/build/drupalFile.js
this.linkUI.actionsView.once(

There is now $.once() anymore in D10.

I went with this drupalFile.js for now (the old one in red)

https://git.drupalcode.org/project/editor_file/-/merge_requests/4/diffs?...

Do you have the link plugin enabled in you CKE format? I'm wondering if we may miss something in the config form to require it. Not much time right now to investigate, but I guess we currently cannot have a toolbar with a "add file" button but no "add link" button.

I believe I do. It's the last one, right?
→

Yes. On my side I use the Linkit module, it may explain why it works for me. Anyway, there's obviously something to fix.

Yes. On my side I use the Linkit module, it may explain why it works for me. Anyway, there's obviously something to fix.

I tested it on a new built site as vanilla as I could get it quickly to make sure it's not some other module / plugin interfering and it's still happening so I'm assuming it's something which has to be fixed here. Although I don't understand how @timohuisman tested it on D10.1.2 without any issues. *head scratch*

The only thing I could find out is that this issue only seems to arise if you add the file upload functionality. It doesn't happen if you only add the image upload button (I believe this plugin also adds the upload functionality to the image dialogue right - I might be wrong?).

Both websites I tested it on run D10.1.2, PHP 8.1, nginx/1.23.4 if it's of any significance.

Setting status based on #47. The added patch is not correct approach, it just uses #44 change to #39.

Patch #50 is working for me. Thanks for the work!

From manual testing, I can confirm that, right now, the patch being built in merge request #4 (current state at https://git.drupalcode.org/project/editor_file/-/merge_requests/4.diff) works when the Linkit plugin is enabled, and doesn't work otherwise (TypeError: this.linkUI.actionsView is null).

I just made a commit which fix the problem on my test website. The current version of https://git.drupalcode.org/project/editor_file/-/merge_requests/4.diff is testable.

@daften: Could you please marked the resolved threads as resolved on https://git.drupalcode.org/project/editor_file/-/merge_requests/4#note_1...? And if they are all resolved (right now it looks like they are), mark the merge request as ready?

Hi GaelG,

I checked, and I can't mark any threads as resolved for some reason, I don't understand why. I checked all threads and saw you checked it already, so I marked the MR as ready, does that help you to move forward?

If you have any idea why I couldn't mark threads as resolved, please let me know, I'm very curious to know if I'm missing something or made a mistake somewhere.

@daften. Thank you! It should make it easier for the maintainers to merge the request, when the issue is reviewed. The thread creator may be the only person allowed to close a thread? Nevermind.

I have MR4 a go against 2.x-dev and it works well. I'm RTBCing this as it seems like it's good to go. The upgrade path after that was seamless, thanks all who worked on this.

Hi all, I've just tested the MR4 diff on 2 projects (D10.2.1 + CK5), and got editor_file working without any issues.
Love to see it merged into a new release! Thanks a lot for the hard work!

DuaelFr → closed merge request !2

Thank you all for the hard work here! <3

I found a bug when using this with linkit and editor_file together, you cannot remove the link of an internal link. The href is removed but attributes are staying

    <a data-entity-uuid="8d3939ae-89a7-4d5b-acf4-caf039a5ed8e" data-entity-type="node">link to home</a>

I'll make a new issue.

THink i fixed it, could use some help in reviewing. 🐛 Editor_file+Linkit: removing links broken Needs review

Automatically closed - issue fixed for 2 weeks with no activity.