🇩🇪Germany @jurgenhaas

jurgenhaas → created an issue.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

This is already the case. Modeler API needs to manage what's in entityTypeBuild because only Modeler API knows about all the context that required to build the collection, i.e. which routes do exist and what permissions are controlling them. This is the case especially when there are multiple modelers available, because that results in pretty complex route management and related stuff.

However, you don't lose any flexibility, instead you just don't have to deal with all that repetitive stuff that every model owner plugin would have to do repetitively over and over again.

Modeler API still allows you to own the edit-form route and build your own edit form like you did before. And within that form you can embed a modeler by calling something like this:

$form['canvas'] = \Drupal::service('modeler_api.service')->embedIntoForm($form, $form_state, $entity, 'bpmn_io');

This will embed the modeler at that place, and the modeler API even handles form validation and form submission for that part of your entity edit form, without you having to bother about anything. Your model owner plugin needs to implement certain things in a way that it just deal with that limited scope, but that's pretty straightforward. We had done that for AI Agents originally, where all the metadata, prompts, etc. have been handled by the regular edit form, and all the tool and sub-agent setup has been delegated to Modeler API in an embeded canvas.

jurgenhaas → created an issue.

Nice finding, thanks for the heads-up @marksmith

I've opened an issue there to ask them for security coverage. Let's see if that happens, then we can add that one too.

jurgenhaas → created an issue.

Nice work. I've added the category to the ECA field widgets and the result is exactly as expected. Thanks you so much.

Merging as this is an onvious bug

Merging as this is an obvious bug.

General question: shouldn't each module that requires an icon, bring that themselves? Gin wouldn't be able to add icons for all sorts of contrib modules with similar requirements.

Thanks @melodia40943 for reporting this, but how does that relate to the Gin theme? It doesn't define anything about cloudflare nor does it require remote resources. It looks like the warnings you're seeing are from something else, not from Gin. Or am I missing something?

The MR contains lots of unrelated changes that need to be reverted first.

Unfortunately, we can't add a workaround for this in Gin. The action button in Mailchimp is not done in a way that matches conventions that are used in Core and most contrib: the submit button needs to be of type submit and should be a child of $form['actions']. I'm moving this to the mailchimp project and propose the fix in an MR.

Thank you everybody.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

Great finding, thanks for fixing this.

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

OK, PHP 8.2 has also started to be more strict. Not as much as 8.3 yet, but still can cause this exception.

The message you found in dblog is something different. It informs you about a deprecated action plugin that will be removed from Drupal core eventually, and if you use that somewhere, you should replace it.

The breakpoint thing is not within the browser, it's in the PHP code, and it requires xdebug to work with it. I haven't made that clear in my original comment above. It's fully understandable that without being a programmer, this is something alien, and you should not be trying learning that just because of this issue.

So, we need to find somebody who tries to reproduce this and then to find out which of the plugins in commerce is causing this issue. For that, it's necessary to have a complete list of all the commerce modules that are installed and also the version of them as well as the versions of Drupal core, ECA, and bpmn_io

This is looking good now, setting to "Needs review" so that people can test and review this.

@mgifford just FYI, the npm build command needs to be run in the Gin directory. But @siddharthjain has now done it for us, thank you.

This is most likely on PHP 8.3, am I right? That's where parameter types are checked much more strictly and if a module provides arguments to config forms that are unexpected, then PHP in the past only logged a warning, but with 8.3 the behaviour can be more impactful.

What you can do to narrow down the origin of the problem is to set a break point in the code where the error occurs, and then try to find out which plugin is providing a value that's unexpected by PHP. When we know where this is coming from, we can work on a strategy to prevent that from happening in the future, and maybe also provide a workaround to heal the issue on the fly.

We've seen a few edge cases in the last 18 months when we had to add special cases for sticky action buttons. Maybe this is another one. I can have a look, potentially Friday. I wouldn't be too worries about a wider impact, as we have addressed this in general and sometimes specifically.

This got removed as part of ✨ Modeler API: Allow users to switch between the model and the form Active .

This is ready for a review.

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

I stand corrected, you're right @mxh. The event is instantiated once for a specific entity, but multiple ECA models can respond to that.

In that case, the implementation of the optional default in the configured event in the ECA model should follow the same logic as what we currently have with the \Drupal\eca_access\Event\EntityAccess::setAccessResult implementation: if the event defines a default (which is not mandatory), and no result is being set yet when getAccessResult() is being called, then the default should be applied in the same way as currently in setAccessResult.

Unless I'm missing something, that should allow for the required flexibility. As for the example in #8, the ECA event which gets triggered first, doesn't set a default, but the second one does. That means, the access denied used as default in the second event will only be applied if nothing has been set at the very end of the access check.

Each of those events will be able to set an access result. How the access control handler deals with that when it receives one after the other, depends upon the implementation of that handler.

I'm not seeing any problem with patches from Drupal.org. It's more a question of preference; in any case, someone could download the patch and commit it locally if they prefer.

There is no assurance that patch files do always exist on d.o - and the DA has always insisted in people not requesting patches from there. Soon, when the migration to GitLab will be completed, all the patch files will most likely be gone. That's why everybody is asked to keep their required patch files locally in the project that requires them.

It will help as discussed in https://drupal.slack.com/archives/C0287U62CSG/p1751989605459709

The idea is that the event can be configured with a default which will be used only if the actions following it won't set any access result. That way, the "forbidden" in the event would only be set, if the following actions didn't set any other decision.

A hotfix release of this fix would be greatly appreciated if possible

Will come soon, just coordinating with a couple of other fixes here and in Gin.

Here's a patch made from both commits fixing the issue.
Posting it in case someone needs to apply it via composer (and doesn't want to switch to dev version).

@abramm Do you know that this is bad practice and you should never grab patches from d.o with composer? You should always download the patches locally and refer to them there.

Please work on the 3.0.x branch, and we'll then backport from there.

Also, when this gets implemented, it should be done in a way that it works for all events that are available for access control.

This is ready for a review.

It's actually not only the cancel button module and ECA, there is also Gin in the game which does a lot of magic to expose action buttons in the top-right corner of the sticky area. And even that will soon change a lot, as in Drupal core there will be the new navigation module and the toolbar module being removed. By then, Gin will no longer have to handle the sticky action buttons, and that'll change this whole behaviour too.

That said, I'm not sure if it's worth trying to figure out which component would have to be adjusted, could be all 3, actually.

I'm open to this approach. This callback has often caused headaches. That said, I'm uncertain about the consequences when removing this. I've never been deep into this and have no idea how much this removal would break out there in the field. @mxh do you have a clue what that would mean?

That's great, thanks for extending the ECA ecosystem.

You've already defined the ECA ecosystem in your main project page which automatically adds it to the list here: https://www.drupal.org/project/eca/ecosystem?page=1 →

Every now and then I'm going through that list and add the new modules manually to the front page of ECA.

What's required is to run npm build locally in your dev environment. That will compile the changes into the distribution files and the result should then also be committed to the MR.

I guess so. It'll be ugly, but it'll work. Of no objection comes up before than, I'll fix it tomorrow.

I'm with you. That would require a change in core, though.

That sounds like the issue reported at https://github.com/Elephant418/Markdownify/issues/47

You should apply the patch from there, then you site should work again.

It's not an issue is this module, though.

Tests are failing because the change was made in the dist/css directory, which contains files that get generated by a preprocessor. Changes need to be made in the styles directory and then npm build needs to be called which updates the distribution files.

Also, color codes are using lower case characters.

Can't find what's going wrong, raised the question in their forum: https://forum.bpmn.io/t/removing-a-template-from-a-sequence-flow-doesnt-...

jurgenhaas → created an issue.

This is great. I wonder if it could even be more optimized such that the returnSuggestions method only returns a key/value array and the whole ajax implementation is done by the base class. That way, the implementing developers have even less work to do, and we only maintain the ajax part once.

I've removed the 2 files and added an update hook to enable the modeler_api.

jurgenhaas → created an issue.

Am I right that this patch is all that's needed to fix the issue? It sounds like, and I'd be happy to merge this if we could get an RTBC on it.

Maybe some clarification required:

When you're on Gin version 4 together with Gin Toolbar 2, this is also using the toolbar module from core as that's declared as a dependency. Only in Drupal core 11.2 together with Gin 5 and Gin Toolbar 3, the core toolbar is no longer required. Instead, the navigation module from core is taking over both the left and the top toolbar.

What we have in Gin 4 is an experimental implementation of that navigation, which is no redundant as navigation module in core has matured, and Gin will remove its experimental implementation as a result of that.

Well. Gin is styling those buttons and they work everywhere. And obviously Gutenberg brings something along, where they don't any longer.

In the past, Gin has incorporated many "quick-fixes" for contrib modules, but we will no longer do that. In fact, all of those will be removed from Gin and module maintainers will have to make sure, that their modules don't break the admin theme. We will open issues with each of those modules soon to let them know what needs to be done, and when Gin moves into core by the end of this year, all those adjustments will not come with it.

That MR!633 needs to go into gutenberg, not into Gin.

Is this about the toolbar module in core? If so, that's deprecated and will be removed soon.

Beyond that, I'm not sure this is a theme issue. Gin doesn't provide the content, it styles the available content. So, if there's a link that's provided by a module that shouldn't be there, the issue should probably reported there. Or am I missing something?

There isn't anything to review here, and the issue is assigned, so that usually means, nobody else is likely looking at it.

I guess it would help if you could provide more details, especially why this should be a Gin issue and not a gutenberg issue.

This is an interesting one. Looking into the 4 entity type forms from core

      'node_type_add_form',
      'comment_type_add_form',
      'media_type_add_form',
      'block_content_type_add_form',

they're all built similarly, but only the media type form behaves differently; the other 3 work as expected. This is because this media type create form wants to support browsers with disabled javascript. Therefore, those buttons are changed from type submit into type button. But Gin currently only treats submit type buttons as sticky buttons with the special treatment in the top bar. And that's important that way, as regular buttons usually have a different behavior and don't submit a form.

The relevant code in core which is responsible for this can be found at \Drupal\media\MediaTypeForm::actions with a comment that explains what's going on.

I'm uncertain how we should resolve this, TBH. The related Drupal core issue is #3018539: Media types cannot be created in the UI without JavaScript → and it's in core since 8.6 in 2018. It probably never occurred to anyone before, as creating new media types through the UI is probably the least that most site builders ever do.

What are our options?

• Treating all buttons as sticky submit button? Maybe comes with unexpected side-effects.
• Adding special treatment in Gin just for this form
• Revisiting the core approach of handling this
• other options?

The sticky buttons in that form have the data-drupal-selector="edit-save-continue" attribute, but that should be data-drupal-selector="gin-sticky-edit-save-continue". I'll investigate how that's happening.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

jurgenhaas → created an issue.

That's technically possible, but I advise strongly against it. That's very bad practice and diggs a massive security issue into the Drupal site that does this.

There have been a few discussions around that token here in the issue queue and also on Slack: it is not a public token that's available like other tokens. Drupal core has implemented that token in an isolated context such that it's only available in the Drupal core context. That's for security reasons. Imagine that token where generally available and somebody puts that into a comment. That would output that link to reset a password in public for everyone to us it.

Therefore, if you want to implement a user creation workflow which is different from Drupal core, you have to build the whole workflow yourself, including the email text that's being sent. You can e.g. send people to https://www.example.com/user/reset and ask them to use that form to receive the password reset link from there.

jurgenhaas → created an issue.

Well, ECA provides granular actions, and the developer of the model is responsible to take all the steps required for any given workflow.

So, in this case, after creating the account, the same ECA model should just add another step to send an email, and the model needs to decide what goes into that email.

What I don't understand is what you mean by activating the account? Drupal's registration process doesn't have anything like inactive accounts that a user can activate with a link. Instead, you need to create that account and make it active. Then, when you send the email with a link, there the user can reset the password and then login.

@pameeela thanks for raising this, I don't really know how to go about it. To answer your question, there hasn't been any progress and nobody seems to be working on it.

However, I still think that what we have in Drupal CMS is exemplary, and the idea of a manifest was born out of the idea that this could be beneficial for marketing Drupal. So, if we see any value in doing so, we should probably push this forward. Otherwise, let's close it for now.

@alex.amtr this issue was closed 2,5 years ago. If you have a follow-up issue with anything addressed in closed issues, please always open a new one.

Thank you @alex.amtr and @anaconda777 for your feedback, glad it's working.

jurgenhaas → created an issue.

I guess you meant 2.1.10

That looks like you've updated eca_group to the latest version but not ECA. Please also update ECA to the latest and this problem should be gone.

Please provide more information on what that ECA model does and how you've set it up.

The action throwing the error is from webform module itself, not from this integration. It may be that it expects the webform as a parameter which is not provided by your model.

jurgenhaas → created an issue.

Let's see if there's any feedback from the community whether this is really an issue and if there are any suggestions on how to resolve it.

Most of these effects have been resolved by other issues, and the remaining focus issue gets addressed in 📌 Off-canvas-wrapper sometimes steals focus Active

jurgenhaas → created an issue.

jurgenhaas → created an issue.

Problem was that every time an element without an assigned template got selected, we registered another event to assign a template when one was chosen in the template chooser. That also caused the wrong templates being assigned to elements that we saw in 🐛 Pasting elements from clipboard is not creating unique new ids Active .

This is now solidly solved.

Added an icon and prevent selection handler from working when search is active.

Seems like we've covered this now.

Does Drupal AI need a bot defender/moderator module?

I'd say probably not. Being hit by bots on a Drupal site is due to unethical behaviour of some other AI solution, not the one in Drupal. While it's important to find some good defence, that's outside the scope of Drupal AI, in my view.

The other way round, when Drupal is crawling remote resources, this is what we are controlling, and here we should provide measures that respect guardrails set by the remote side.

This is not a code issue, nor is it an ECA problem. This is how Drupal CMS defined the user registration to work. But we made an assumption that isn't static and can get changed by the administrator of the site.

What we wanted to achieve:

OOTB, Drupal CMS is configured such that only administrators can create new user accounts and that email verification is required. In that context, the create user form from Drupal core has some issues that we solved with ECA, just for that scenario: when the administrator creates an account, it doesn't make sense that they type in a password. Therefore, ECA hides the password field. The user gets notified and receives a one-time login link. Easy for the admin, and easy for the user.

What we also allowed is to turn off notification. If the admin disables that in the user create form, the password field re-appears, and the admin is forced to type in a password.

Up to this point, it works as designed.

What's not covered by the OOTB configuration

If the site changes the user settings and allows self-registration by users, the ECA model should probably keep itself out of the loop, so that the form and the workflow behaves as Drupal core was designed originally. This could be added easily, if that's what we want.

I should note, before we're changing anything here, we should first finalize the tests in 🐛 Incorrect password on login Active , otherwise we're changing things in too many places in parallel.