🇬🇧United Kingdom @nicrodgers

rebased to bring it up to date with the latest 11.x

nicrodgers → made their first commit to this issue’s fork.

rebased the MR to bring it up to date with 11.x

nicrodgers → made their first commit to this issue’s fork.

Here's a screenshot showing an example of the error page that is displayed instead of the nice Drupal themed 404 page.

nicrodgers → created an issue.

I've been testing this out, and the breadcrumbs are still duplicated on views pages. It looks like this is because the fix has been applied within the taxonomy term logic so for views pages it never gets hit and the default route params are still missing. Was there a reason this had to be nested inside the taxonomy term logic rather than being outside so it can apply everywhere?

I found that MR26 doesn't apply if you're using the Drupal 11 compatibility patch in https://www.drupal.org/node/3485520 → due to them both modifying the params in MenuBasedBreadcrumbBuilder::construct().

So i've re-rolled the attached patch, which enables you to patch this issue on top of the other one.

If this issue gets committed before the other one, then use the MR.

Setting back to needs work due to the failing coding standards issues picked up in the pipeline.

I've updated the issue summary, and created a merge request based on the patch from #71 as the patches have been deprecated.

Patch applies and upgrade status reports there are 0 issues after applying it.

nicrodgers → created an issue.

@maxilein are you sure you've applied the MR #5?
That is one of the things that's changed in it.

I don't see that code, or that error reported by Upgrade Status after applying it.

@pearls we are still using this MR on our sites and it's working fine:
https://git.drupalcode.org/project/drupal/-/merge_requests/715/diffs

@miiimooo we use content translation and workbench_moderation, so for us having that option would make no difference, we'd still need to use the core patch to improve the speed

nicrodgers → changed the visibility of the branch 3484874-filevalidateextensions-is-deprecated to active.

nicrodgers → changed the visibility of the branch 3484874-filevalidateextensions-is-deprecated to hidden.

nicrodgers → created an issue.

I've updated the MR to declare support for D11, as upgrade status was reporting that everything was compatible with no further changes required. I've hidden the patch files for clarity, the latest code is in the MR.

I've been debugging our case of this further, and it appears that our site is triggering it due to the same code mentioned in https://www.drupal.org/project/search_api/issues/3274158#comment-15802027 🐛 RuntimeException while trying to render item Active when it tries to build a form, it checks if the session has batch_form_state data set. If the session isn't yet started, it tries to start it, but NativeSessionStorage refuses to do so because the HTTP response headers have already been set (by the page's initial response).

This can be triggered in our case either by having the content moderation form show on the page (eg. moderate node from draft to published) or the autosaveform module.

However, unlike the report in #36, in our case, when this exception is triggered, the content is still successfully updated in the index, so it doesn't seem to be causing any real problems for us.

I'm still unsure as to why it would have only started happening since we upgraded from php 8.1 to 8.3 though.

FWIW - For us, these errors only started occurring after updating PHP from 8.1 to 8.3.

I think this can be closed in favour of https://www.drupal.org/project/file_delete/issues/3430549 📌 Automated Drupal 11 compatibility fixes for file_delete Needs review

I ran this patch through Upgrade Status for Drupal 11 compatibility, and all that was needed was adding ^11 to the info.yml, so I've done that. Now this issue can make the module compatible with D10 and D11. Seemed pointless creating a new issue for D11 and doing them separately. Setting back to Needs Review because of that.

nicrodgers → made their first commit to this issue’s fork.

nicrodgers → created an issue.

Patch 11 doesn't seem to be working with the current 2.x-dev version of the module.
The patch applies, but even after rebuilding caches, the additional attributes are not displayed. Only the standard three are displayed:

* target
* rel
* class

You need the D10 compatibility patch as well, from #3289374

Created an MR and attached a patch (same thing) with necessary changes for Upgrade Status to be happy.

nicrodgers → created an issue.

MR 5 applies and works fine with D11

MR15 no longer applies to 2.0.x-dev. I think it needs rebasing.

nicrodgers → created an issue.

nicrodgers → created an issue. See original summary → .

We can't use the DeprecationHelper without dropping support for Drupal < 10.1.3 because that's when the helper utility was introduced. Currently this module says it's compatible with 8, 9 and 10.

So instead of using DeprecationHelper we can do it the old-school way

if (version_compare(\Drupal::VERSION, '10.1', '>=')) {
  $logger = \Drupal::logger('pwned_passwords')
  Error::logException($logger, $exception);
}
else {
  // @phpstan-ignore-next-line
  watchdog_exception('lockr', $exception);
};

or we can drop support for Drupal <= 10.1.3

Duplicate of https://www.drupal.org/project/clamav/issues/3429235 📌 Automated Drupal 11 compatibility fixes for clamav Needs review

nicrodgers → created an issue.

Support from all Views contextual filter settings for the default_argument_skip_url setting is removed from drupal:11.0.0. No replacement is provided. See https://www.drupal.org/node/3382316 → .

We need to remove this from

config/optional/views.view.track_role_grants.yml
config/optional/views.view.track_role_history.yml

nicrodgers → created an issue.

I've fixed the 'not exists' typo and also added 'the' as a prefix to 'filesystem' to improve readability.

nicrodgers → created an issue.

Certainly is from us! Surprised other people haven't shown interest in it though.

Merged - thanks!

~2dareis2do - We are still using the patch from #30 in production on 10.3 . Haven't tried the newer patches.

I've re-read through every comment in this issue and have observed the following:

data-drupal-link-system-path

in #4, @nod said

li element shouldn't have the data-drupal-link-system-path either.

in #94, @andrewmacpherson said

What's going on with the <li data-drupal-link-system-path> attribute? In #4, _nod suggested removing it from the list item. It's still there though. It looks like an earlier patch (#26) removed it, but by patch #82 it's come back.

This hasn't been addressed, so we need to decide whether this is something that needs to be done here or not, and update the issue summary and description accordingly.

data-drupal-language

in #94 @andrewmacpherson also said

The new data-drupal-language attribute isn't very well explained.

and this hasn't been addressed yet either.

failing tests in #118 need addressing

needs re-rolling

I've tested MR 29 with Drupal 10.2 and can confirm it displays the helper text correctly, validates correctly and enables svg images to be uploaded.

I have not tested with Drupal < 10.2.

I have hidden MR 27 to make it clearer which MR is 'current'.

I reviewed the changes for coding standards, and have pushed a bunch of fixes to bring the files in line with Drupal coding standards.

I am not sure why so much code has been removed in the SvgImageWidget class, relating to settings and preview - is this intentional?

nicrodgers → changed the visibility of the branch 3413668-replace-upload-validators to hidden.

nicrodgers → made their first commit to this issue’s fork.

Thanks for the patch.

Is it possible to do that patch so that it works with both versions of scheduler?

If the patch fixes it with the latest version of scheduler, presumably it will break for people still using the previous version?
So to avoid releasing a new major version of advanced_scheduler I was wondering whether it was possible to make it work with both.

If not, we can roll this in to a new major version.

Doesn't apply to latest 2.x-dev - needs re-roll

Good spot Michalsen, I've removed those two now.

Removed the two unused Use statements.

This is still occuring on the latest dev version. The problem is that it assumes all SearchPlugins implement configuration, but actually only those that implement ConfigurableSearchPluginInterface do. Help search pages and others cause this error.

I was also occasionally getting the error ' The theme implementations may not be rendered until all modules are loaded. ' whilst testing this, which is why I've added the call to \Drupal::moduleHandler()->loadAll();

nicrodgers → made their first commit to this issue’s fork.

nicrodgers → created an issue. See original summary → .

It's been a while since I worked on it, but from memory I think the sort is required because there's no guarantee that the order of params matches, so unless you sort them you could have a different order and thus it wouldn't match (even though the order is irrelevant).

xurizaemon → credited nicrodgers → .

I've spotted a bug with this patch - I don't think it's specific to our site, but if someone else could test it and confirm if they can replicate it too that'd be great.

Steps to reproduce:
1. Go to /node/add/page
2. On a field that uses Ckeditor5 (eg. body field), change the default text format from Basic HTML to Full HTML
3. Enter some text in the body field
4. Save the node

Actual Result:
The body field text is not saved or displayed.

Expected Result:
The body field text should be saved and displayed.

Notes:
It doesn't seem specific to the Full HTML input format. The problem seems to occur simply by changing the text format from the default on page load to anything else. For example, if the default input format is Full HTML, you can add text and it works fine. But if you change it from Full HTML to basic (before entering any content) then it doesn't save or display.

Thanks @Liam. I've changed the target branch to 11.x
Setting back to Needs Review. We need people to test with latest firefox versions (see #14)

@MegaChriz
Thanks so much for your help. I've managed to configure the toolbar with the buttons I want on each different text input format.

The styles dropdown is empty however, it's not picking up styles from the site (like it did with ckeditor 4). Has anyone managed to get this working?

Thanks @MegaChriz - that's really helpful. I'll give it a go. How do I find out the 'toolbar_conf' value for each plugin in ckeditor that I want to reference?

For example, 'bold' and 'italic' are obvious, but how would I find out the value for 'inline image' or 'media embed' or 'table properties' ?

Is it possible to use multiple customised editor builds, so that different toolbars can be used for different text input formats?

For example, a simple one on 'basic HTML' and a more complex one for 'Full HTML'?

Turns out it was an issue with our theme.

Previously, in hook_preprocess_field we had $variables['items'][x]['content']['#options']['attributes'] but now they are in the url object's attributes.

nicrodgers → created an issue.

nicrodgers → created an issue.

Thanks!

good idea - thanks!
Just one comment on the MR, once that's sorted we can get this merged.

Thanks, committed.

Thanks, I've committed your fix. Apologies, I forgot to check the credit before committing and it was omitted.

Fixed - thanks!

nicrodgers → created an issue.

nicrodgers → changed the visibility of the branch 3420162-fix-phpcs-issues to hidden.

nicrodgers → created an issue.

nicrodgers → created an issue.

nicrodgers → created an issue.

Thanks for testing, committed!

nicrodgers → created an issue.

eg https://git.drupalcode.org/project/msacr/-/jobs/774873

nicrodgers → created an issue.

nicrodgers → created an issue.

Changed MR destination branch to 2.1.x, the tests were failing anyway so I'll clean that up elsewhere and come back to this.