Drupal.ckeditor5.openDialog missing existingValues param

This can potentially break contribs that calls this function

Yes, that's right. Although I wonder how many contribs actually use this. TBH, I only discovered this method due to this bug. I couldn't find any documentation on this. Neither any info on the related Drupal.ckeditor.openDialog.

Especially due to the missing existingValues param this method (in contrast to the version for CK4) is almost useless, I guess. I resorted to a direct Drupal.ajax() call for now.

@mglaman Any chance you could write a test? 🤞🙏😊

Yeah, I'll see what I can do. I'll pick this up for Monday.

Why not make existingValues the last parameter and default it to {}? This way we won't break any contrib calling this method without it

ckeditor5.es6.js has been removed since the previous patch

Also attaching the Drupal 9 patch version which includes ckeditor5.es6.js

The only argument against it would perhaps be the symmetry to the CKE4 version. But I don't really oppose the idea. Just a thought.

I see 2 options here:
- keep the CKE4 parameters order in sake of the symmetry. Expect issues with the contrib modules calling Drupal.ckeditor5.openDialog. Not sure if there is a way to evaluate how many of them actually do that. Anyways this approach would require collaboration with the maintainers of each of those modules
- move the existingValues param at the end and default it to {} as I did in the patches #11 and #12. Given the difference between CKE4 and CKE5 I think that the parameters inconsistency is less evil than having to adjust contrib modules

One way or another I needed these patches in order to make the insert_view_adv contrib module work https://www.drupal.org/project/insert_view_adv/issues/3356931 🐛 CKEditor 5 plugin doesn't default to the selected view when editing Needs work

I'd be in favor of option 2.

Anyone who has used this function in CKE5 so far obviously hasn't used existing values or else would have needed a patch/custom solution, so better to keep the function backwards compatible

The primary thing missing here is test coverage. Any takers? 😊🤞

Just adding that there isn't currently a test for this without the patch. Currently only the media library plugin calls this function in core (see core/modules/ckeditor5/ckeditor5.ckeditor5.yml)

#17: valuable contribution, thanks 😊 That raises another question: why does the Media Library CKEditor 5 plugin in Drupal core not run into this problem? 🤔

@Wim Leers. Very good question. I asked myself the same thing. The answer is: The Drupal Media Library plugin has no edit feature. If you click on the "Insert media" button a dialog is opened and you can select a media file. The media file is then inserted at the cursor position. However, there is no real way to "edit" the media file. You cannot double-click and if you highlight the media item and click on the button again, just normal dialog is opened again but the current file is not preselected. It's okay-ish I guess for this use-case. It would be nicer though, if the library would preselect the file. If you use sth. like media_directories this gets worse because you have to select the same folder again. Long story short, there currently isn't any information flow from CKEditor to the dialog - only from the dialog back to CKEditor. That's why this feature isn't needed.