🇩🇪Germany @szeidler

I encountered the issue with hierarchical terms. Whenever I search for a term, that has parents (so is nested further down the tree), I get a Notice: Undefined offset: -1 in Drupal\taxonomy\Form\OverviewTerms->buildForm(). So it tries to find element -1 in an numbered array, which does not exist. In addition does the term appear twice (with the same ID).

Here's an updated patch against 10.2.x

Is the module still active?

According to the Mailchimp API documentation https://mailchimp.com/developer/marketing/docs/errors/ all "Invalid Resource" on subscribe should be validation issues on the Mailchimp end. The MR here prevents logging those into the Drupal logs at all.

I agree. It's still an issue in 2.x-dev. We're getting a lot of those "errors" in our logs, which creates a lot of noise.

In this case exposing it as an error makes sense from the Mailchimp API perspective, but for the Drupal module it's not more than a warning and should be logged like this.

Could you check, that the "Media" Gutenberg block is enabled in the content type's Gutenberg configuration?

If that is not the case, then it seems that the 2.x => 3.x update was not running properly or there was an issue in your config-workflow, so that it might have gotten reverted.

szeidler → created an issue.

szeidler → created an issue.

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

There is a work around mentioned but I don't think implemented by this module. I found it simpler to add a user field and use SSO. Though in my case both fotoweb and Drupal were already using SSO so YMMV.

Yes, that's also the combination we used before switching to the oAuth2 authenticate type. But in general you're right: When using the old "FW Token" authentication method, SSO is not strictly required, so you merge request makes absolutely sense.

Using the MR on a project for quite a while. Seems to behave as it should. Thanks for the contribution @eiriksm

I still have problems to see how this can work. getGutenbergNodeTypeFromRoute

if ($node_type) {
      // Then just return it.
      return $node_type->get('type');
    }

$node_type is group:request for me and since the condition does not check if it's an NodeType object and fails with

Error: Call to a member function get() on string in Drupal\gutenberg\GutenbergContentTypeManager->getGutenbergNodeTypeFromRoute() (line 120 of modules/contrib/gutenberg/src/GutenbergContentTypeManager.php).

No, you're right. Since it's not an alter, but invokeAll. The the approach is actually absolutely alright.

The problem indeed exist. Without a return nothing will happen. But why do you have the assumption in your patch that hook_gutenberg_node_type_route() returns an array? Shouldn't that return simply the content type as a string?

We could of course have added a table exist check on the update hook, but I'm just surprised that the table does not exist for your since it's defined as hook_schema + was added as an update hook for existing sites years ago.
Do you know since when the site has been using Gutenberg? Have you been an early adopter?

We're running into similar issues with multiple modules, mainly when they perform service changes as part of an update.

But that seems to be drush or core responsibility. You can have a look if https://github.com/drush-ops/drush/issues/5952 fixes the problem for you. Most likely will get into the next release.

Thanks for providing the MR, this is a really annoying issue. Should we implement a update hook that removes the stored config value for existing sites?

There is still something wrong on the file URLs on your patch #16

For our case the merge request is fixing the problem, whereas #12 does not.

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

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

Created a MR and rerolled my changes. It seems going via the interface methods from entityqueue solve all of the old custom handling by design. I tested all combinations: queue limit, reverse, act as a queue and everything seem to behave correctly out of the box. But you should give it a test as well.

Added a merge request for the change.

szeidler → created an issue.

We are experiencing the same issue after start using the module.

Looks good now 👏

That looks pretty good. Could you fix the coding standard issue, that got introduced by your change.

https://www.drupal.org/pift-ci-job/2861167 →

line 43	A comma should follow the last multiline array item. Found: 'manual'

The other ones can be ignored.

In Drupal 10.2 we have a new route for the new modern Field UI. That one is defined with field_ui.field_add_$ENTITY_TYPE_ID

Here's a patch that adds the new route to the $adminRoute check in the social_media_link module.

Has someone tested this out when using simplytest.me and have been able to reproduce? I'm using the module with language path prefix and it seems to be working as expected and does not add any query string.

Thanks for your detailed issue description, especially @MrMiso.

There must have indeed been a change in the language switcher rendering of links in Drupal 10, so that a link without the `#url` property is not rendered at all anymore.

Luckily there is a `<nolink>` route that can be used for achieving what we wanted. It seems that all your reported failures have been related to the wrong `<nolink>` handling.

Could you give the following patch a try?

Fixed in the 2.0.3 release.

szeidler → created an issue.

The status has been set to `Maintainer needs more info`. What kind of info is requested here?

The problem still persist for us. Any require_once and include_once are misbehaving for us. It's not a big deal, we can easily change it to the without `_once` function. Not sure if it is drush or Drupal core causing it.

Thanks for your work on this. I also struggled with the same issue. In addition we have a use-case that we have translated content and the translations could have a different published status. A bit like described in #2469523: Add language support to node access grants → . When using the patch as it is here the source language of a content would decide if the content is treated as published or unpublished and react wrong on the translations, if the status is different.

I added language capability to patch #14 and extended the tests. Please decide if that should go into this issue or handled separately. If not, then ongoing work should be based on #14.

Thanks for a review.

It has been fixed in ✨ Support "Clone" functionality Fixed and is part of the dev release (not part of the recent stable release).

Although it was targeting another issue, it's solving this one.

Here's a reroll for 10.2.x . Note, that has been a change committed on that branch (after 10.2.0 release) that makes the overview page not load the whole taxonomy term entities anymore, but uses the terms as generic php classes, which needed some adoptions in the patch 🐛 Avoid loading all terms on the taxonomy overview form RTBC

This patch does not work with 10.2.0, but will work with 10.2.1 (unless there are more changes to come).

Rerolling #292 for Drupal 10.2

szeidler → changed the visibility of the branch 3408193-z-index-of-ui-dialog to hidden.

szeidler → changed the visibility of the branch 3407744-fotoweb-media-library to active.

szeidler → changed the visibility of the branch 3407744-fotoweb-media-library to hidden.

Here's a patch.

szeidler → created an issue.

The $event_dispatcher->dispatch arguments have been defined in the wrong order. Here's an updated patch.

Looks good for me and is necessary now in PHP 8.2 times.

szeidler → created an issue.

Make it Drupal 10 compatible.

Make the patch Drupal 10 compatible, because Event in Symfony got deprecated and replaced with an own Drupal event, available since Drupal 9.

https://www.drupal.org/node/3159012 →

Yes, the problem is tied to Drupal 10.1 where the support for AJAX GET requests has been introduced and made the default for example pagination.

It's essentially about this line.

if ($q === NULL && $this->request->isMethod('POST') && !empty($_REQUEST['view_path'])) {

I don't know why the limitation to the method was made, but in my test case changing it to the following resolved the issue

if ($q === NULL && !empty($_REQUEST['view_path'])) {

With Drupal 10.1 views is using GET AJAX requests by default → , when AJAX is enabled.

In my case we're using views_infinite_scroll and the path context is lost, because of the POST limitation in this commit. I think we would need to make it work with GET requests as well now.

I was fighting with the same problem as well. Rerolled against recent 3.x-dev + included feedback from #6

We see a similar behavior after having upgraded to Drush 12.

We also include a different file in our settings.php. We needed to replace our include_once with include to make it work.

Unfortunately is the CKEditor Plugin not maintained anymore and the translation service from the university not available anymore, which makes the module here basically not working anymore.

Merged.

szeidler → created an issue.

Thanks for spotting the issue. Yes, the `local_image` property actually should be removed.

There seems to be still some uses for the local_image property on the media thumbnail. That seems to be needed to be refactored/removed as well. https://git.drupalcode.org/project/media_fotoweb/-/blob/2.0.x/src/Plugin...

Unfortunately my testing is currently limited, since I have no customer which grants me local access to their Fotoweb installation and Fotoweb so far didn't wanted to provide me a demo/sandbox access to be able to properly independent work with their API and selection widget…

It is actually related to a Core issue #2635728: Uninstalling a module providing display extenders causes fatal errors → and might happen with other modules (that provides text filters) as well.

In #3204782: Uninstalling Gutenberg editor broke site → we implemented such a routine, but unfortunately the Drupal core config validation (your error) is triggered before gutenberg_uninstall(). So manual steps would be to disable the Gutenberg text filter on all text formats that are using it (most likely only the Gutenberg text format) and afterwards uninstall the module.

We have been hoping for bigger attention on the underlying core issue, but it ended up in a new Gutenberg release 8.x-2.8 now :)

szeidler → created an issue.

There's a 2.0.x with those changes now.

szeidler → created an issue.

If I understand it correctly is the language switcher block using the native language names, isn't it? That would mean we could just set it to the language code of the language that is supposed to be rendered as a switcher link? Which essentially also should always be the same as the hreflang attribute value?

Then we would need to go into all classes implementing LanguageSwitcherInterface and add the attribute there, similar to this.

foreach ($this->languageManager->getNativeLanguages() as $language) {
  $links[$language->getId()] = [
    // We need to clone the $url object to avoid using the same one for all
    // links. When the links are rendered, options are set on the $url
    // object, so if we use the same one, they would be set for all links.
    'url' => clone $url,
    'title' => $language->getName(),
    'language' => $language,
    'attributes' => ['class' => ['language-link'], 'lang' => $language->getId()],
    'query' => $query,
  ];
}

Is it easy as that?

Here, you have discribe the "native behavior" between media & crop.
To be honest, I did'nt have test this module, I have juste check code, and realise, we address the same point.

You're right: it's the native behavior with Media & Crop. But it is not the native behavior when using the Media Library Media Modify Focal Point module in addition. Then (at the time the module was created) the focal point position was not stored at all. When using the Media Library Media Modify Focal Point module it will store the position as the global media focal point position.

I guess both module solutions (purely contextual or global) have their use-cases.

Here's a patch for the description change of the tokens. We could decline open merge requests for this issue then.

The existing descriptions are outdated - please could you change them?

'The URL of the page to add a subscription.'
'The URL of the page to remove a subscription.'

I hope I understood it right and changed the correct token descriptions?

Patch seems to have missed the an empty line at the end. This one should work. Unfortunately testing is not enabled for the 2.0.x branch. Could you please test?

I agree with you about the tokens. Then the remaining part would be just the clarification in the description, right?

Thanks for splitting up the issue and providing the patch. It fixes the issue.

AdamPS → credited szeidler → .

Here's an implemented KernelTest that runs those test migrations, one for a text field on a node, one for a base property on the user entity.

szeidler → created an issue.

Here's a try. Thanks for reviewing.

szeidler → created an issue.

Hi,

yes, that is a simpler way and is working. Although a dedicated token acts a bit more like a reliable API for the ones using it. Let me know, what you prefer.

Great, I haven't thought about it, but that's pragmatic.

I created a new merge request against 4.x including the tests. I catched another (so far untested problem) on the ConfirmationController, when the immediate option was used: https://git.drupalcode.org/project/simplenews/-/merge_requests/33/diffs?...

Here's a patch against 3.x that demonstrates the problem described in #5.

I haven't looked into the 4.x-dev branch, but for me the merge request led to wrong hashes, so that a confirmation was impossible. The hash function in the token vs. the one in the controller mismatched.

Fix it in the initial merge request.

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

Patch still applies for 2.x. In most cases users have control about the processor configuration. I had an edge-case where I wanted to manipulate the HierarchyProcessor, which is not that easy to do. For that the alteration of plugin info is quite helpful.

If I understand your request correctly this is what you're looking for.

All tests are passing green now. Your changes are doing, what they should. Happy caching.

Thank you for the effort!

Fixed.

szeidler → created an issue.

Thanks for the contributions. I can confirm the problem.

I enabled testing on the 2.x branch, but we seem to have an issue with the 3rd party dependencies and Guzzle in Drupal 10. I should have had enabled it earlier…

I queued another test with Drupal 9.5 now, so that the Drupal 10 Guzzle issue can be tackled individually and we can resolve this task as soon as possible.

Thanks for your contributions. This has been committed to 8.x-2.x-dev.

Thanks for the contributions. This has been committed to 8.x-1.x-dev