Fix scandir error in Drupal 10

This worked within the patch for me...

$files = scandir('modules/ckeditor/vendor/lang');

I've modified the patch to use the module list service.

#3 Worked perfectly for me!

This was fixed in #3311367: Fix "Unknown themes: classy" error in the tests → , we just need a new release.

@Berdir, not quite - patch here fixes another occurrence of the core/vendor path + committed patch uses an extra service while there is already injected one

The last submitted patch, 7: ckeditor4-drupal10-compatibility-fixes-3346541-7.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

D10.1.x tests are failing due to https://www.drupal.org/node/3328698 → landed, needs refactor

Patch 11 works great with ckeditor on D10

I've hidden #11. And requeued a test of #7. But like #12, this fixes the issue for me.

The last submitted patch, 7: ckeditor4-drupal10-compatibility-fixes-3346541-7.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Tests are still failing. Obviously still needs something.

@Berdir, not quite - patch here fixes another occurrence of the core/vendor path + committed patch uses an extra service while there is already injected one

@Taran2L, what is he scope of this issue then? Is it about some bug around asset skins/moono-lisa/editor.css (how exactly does that bug manifest itself) or still about a scandir error?
On the surface it looks like #7 is doing two completely different things:

1. a path fix for a specific library asset
2. an internal technical improvement to the directory scan

The title of this issue however is "Fix scandir error in Drupal 10" which indeed appears to have been fixed already in #3311367: Fix "Unknown themes: classy" error in the tests → , although it looks like that could possibly be fixed in a much cleaner way. Not a functional bug anymore, however.

@Eric_A,
the patch is needed because ckeditor 4 was moved from core to contrib

core/assets/vendor/ckeditor/lang is no longer in core.

The patch revolves around this point.

Enable another language for Drupal and you'll likely discover this bug.

@joseph.olstad, the point that @Berdir makes in #5 is that the scandir issue is already fixed in HEAD. (Fixed by Wim Leers, lauriii and others.)

The point I'm making in #16 is that @Taran2L is
1) Cleaning up the scandir fix that is already in HEAD.
2) Fixing a library definition bug. It's not immediately clear to me what that is about. It's not "Fix scandir error in Drupal 10".

I'm proposing to split off either the fix or the improvement to its own scope and issue.

To observe this bug you need to install a second language and then install the ckeditor module in D10 (ck4) , configure a text format to use ckeditor 4 and rebuild caches, visit a node form that uses ckeditor4.

We're using patch 11 with D10.0.9 , I'm not sure how this could be fixed in core when we still need the patch.

@Eric_A, technically it should go to separate issues, but the changes are so small that just fixing it here saves a lot of time imo .. anyways CKE4 is not receiving any significant attention, creating two more issues won't help

@joseph.olstad, are you sure you can reproduce the scandir issue with 1.0.x HEAD? Berdir (and I) are observing that a fix for that issue is committed and pushed to the 1.0.x branch. Of course we can all agree that the issue is present in 1.0.1.

@Taran2L, to get this properly reviewed and to RBTC, comments on the functional manifestation and severity of the *library definition bug* will help a lot. The library definition change from #7 looks simple enough. It's hard for passing by code reviewers to judge wether anything else is needed if there is no information with regards to manifestation and severity.
(The scandir issue on the other hand is very clear to me. I experienced the PHP errors with 1.0.1 and got it resolved by leveraging the pushed fix, mentioned by Berdir in #5.)

To summarize:
In #7 there's code for a library definition bug, but no information on that in the issue title or issue summary.
In #7 there's a very nice-to-have improvement on the existing scandir code. Of course absolutely fine, but at the same time distracting from the library definition issue. Or the other way around.
Clear scoping will get issues much faster to RTBC and RTBC is when committers really start to pay attention.

This affects 1.0.1 , head of 1.0.x hasn't been touched for 8 months, please tag 1.0.2

I have not tried 1.0.x however if it's good please tag a release :)

I'll try to test 1.0.x soon however when I looked it was modified sept 2022 same month that 1.0.1 was published , sorry if I overlooked dev, I should have looked closer at the dates.

With 1.0.1, I had

• Warning : foreach() argument must be of type array|object, bool given dans Drupal\ckeditor\Plugin\Editor\CKEditor->getLangcodes()
• Warning : scandir(core/assets/vendor/ckeditor/lang): Failed to open directory: No such file or directory
• Warning : scandir(): (errno 2): No such file or directory dans Drupal\ckeditor\Plugin\Editor\CKEditor->getLangcodes()

Those 3 warnings are gone with 1.0.x-dev@dev

Tagging a new release would be cool

I don't think we need the changes to \Drupal\ckeditor\Plugin\Editor\CKEditor. Just fixing the library path should be sufficient.

The last submitted patch, 25: 3346541-25.patch, failed testing. View results →

Committed #25 because same failure exists on HEAD. To fix #11, it's going to need its own issue.

Thanks for the tagged release @laurii

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