Committed and pushed to 2.x. Thanks!
Hello @AstonVictor, truthfully I haven't been able to pay as much attention to this module as I initially expected, due to some real life issues.
Today I had an afternoon free for contrib, so I released D10 compatible version of my modules that still didn't have them (This one did, but only on the dev branch. I have officially added a new release that includes it).
I wouldn't mind making you a co-maintainer, as you can see we have some issue backlogs that I never got around to.
If you could help creating some code for getting some of those features/bugfixes in, I would be more than happy to make you a co-maintainer.
But as it stands I don't see your participation in any of the on-going issues, and I'd feel more comfortable giving co-maintainer status if I actually get to see some of your code/issue management in action. I hope you understand.
The D10 compatibility was already on the dev branch (See 📌 Automated Drupal 10 compatibility fixes Fixed ).
I created an official 2.0.0 release that includes it, where all further updates should be worked on from now on.
Hello @frondeau, the patch that was already commited to the dev branch already did not have the core property.
Either way, there exists an official 2.0.0 release now, that you can use from now on, with the D10 compatibility.
Thanks!
gueguerreiro → created an issue.
Thank you @DieterHolvoet, I pushed my changes directly to the branch
Merge request 47 worked perfectly for me, except until I needed to use an in-between date filter, which instead provides multi-date fields (min and max).
The exposedFormAlter() Seems to have special cases for the min/max fields already, but the same does not seem to exist for the exposedFormSubmit() and getElement() functions.
I made some changes to make it work on a project of mine. I'm not sure how to fork from @DieterHolvoet's existing branch, so I'm attaching a patch with the changes (Relative to the MR 47). It can be seen as a diff.
If you could review it and include it on your MR it would be greatly appreciated @DieterHolvoet!
Opened a PR with the one line code change, but I don't remember if we prefer patches for Core changes, so here's that too.
gueguerreiro → created an issue.
It is used there, but it is clearly not supported if we head to the api.drupal.org page: https://api.drupal.org/api/drupal/core!lib!Drupal!Component!Datetime!Dat... It looks like broken text that shouldn't be there.
I'm not sure if there's an issue queue specific for the parser that is being used to build the api.drupal.org pages, but wouldn't @method calls being explicitly supported by the Coding Standards be a pre-requirement to have that support in api.drupal.org also? See #3323252-12: Add @method PhpDoc for EntityStorageInterface descendants → in the related issues, and #3077520-22: Fix and improve DateTimePlus documentation → , which pertains directly to the DateTime documentation with @methods that was linked above.
I agree with #5 that if we're including @method support, we should also add support for @property methods.
FYI, this also happened on one of our websites after an upgrade to Drupal 9.5, which caused CKEditor 4 to break, because of an incompatibility with advagg, which expects CKEditor JS path to be the one from core: 🐛 CKeditor skin CSS 404 after "Enable preprocess on all JS" Needs review . We did not want to use the one from contrib, but there was no way to force it to use the module from Core, that I could tell.
And the migration to CKEditor 5 is still a no-go for this particular website, because there are some installed CKEditor plugins that are still not compatible with the new version.
Opened a PR with the changes. Like I said in my comment, this is a longer PR to account for users that should work for user that are still using Drupal Core's version of CKEditor 4 (So no breaking change), and for users that have contributed modules installed anywhere other than modules/contrib.
The URL for the patch is https://git.drupalcode.org/project/advagg/-/merge_requests/23.diff, if someone needs to apply it through composer.
We were having the same CKEditor 4 issue on one of our websites.
Now knowing that it's because of advagg's "Enable preprocess on all JS" option, I was able to debug this further.
The problem technically happens because of this line:
// Force all JS to be preprocessed.
if ($config->get('js_preprocess')) {
foreach ($js as $path => &$values) {
// However CKEditor must not be combined or errors *will* occur.
if ($path == 'core/assets/vendor/ckeditor/ckeditor.js') {
continue;
}
(.......)
}
There's an exception built in to the "Enable preprocess on all JS" options, specifically for CKEditor, because it leads to errors.
The problem here is that, on some installs, because of 🐛 Remove the "replace" section from core/composer.json Fixed , and the fact that CKEditor 4 was deprecated in Drupal 9.5, it is causing the contributed CKEditor (drupal.org/project/ckeditor) to be installed and used instead. And when that happens, the CKEditor JS file being used will no longer be the one from Core, which means the exception is never triggered (because it explicitly checks for an hardcoded path to the JS file, that is expected to be in the core folder), and the errors occur (As the comment from the code promises would happen).
This is made more complicated because I believe this is only happening on **some** installs. Other installs are correctly still using Drupal Core's composer. I believe this is tied to contributed modules that depend on CKEditor 4. If you have one on your website, and are using Durpal 9.5 or higher, then the contributed CKEditor 4 will be installed. Otherwise you will stay on Drupal Core's CKEditor 4. But I'm not 100% sure on any of this.
With that in mind, in theory, the fix could be a simple change of that line of code to:
if ($path == 'core/assets/vendor/ckeditor/ckeditor.js' || $path == 'modules/contrib/ckeditor/vendor/ckeditor.js') {
continue;
}
So that it supports both cases.
Although, technically, not all users will be using the default "modules/contrib" structure for composer installed modules. So I'm creating a longer PR for those edge cases, that checks for the module path of CKEditor, and if it's not part of the core folder, we use that instead.
Locally, on one of our affected websites, this is working as intended. The CKEditor works even with the "Enable preprocess on all JS" option enabled.
gueguerreiro → made their first commit to this issue’s fork.
Using this PR patch on our PHP 8.1 projects with no issues. Moving to RTBC.
It seems this code change was already commited to the 1.0.x branch: https://git.drupalcode.org/project/search_api_tracking/-/merge_requests/..., but a new release was never made.
Unfortunately there isn't a dev branch published for this module, so we can't even use that to get the latest 1.0.x branch code.
We'd need the maintainer to just create a new release, to fix the issue permanently.