Porta Westfalica
Account created on 9 May 2008, about 17 years ago
  • Drupal Software Engineer & Developer, Project Management at DROWL.de 
#

Merge Requests

More

Recent comments

🇩🇪Germany Anybody Porta Westfalica

Anyone who needs it urgently may switch to 3.x-dev@dev (if project policy allows). Still I think this is an important fix justifying a release.

🇩🇪Germany Anybody Porta Westfalica

Maybe we should first add an issue to add tests for not breaking partial words. After merging that and updating the fork, I guess they will fail expectedly.

🇩🇪Germany Anybody Porta Westfalica

Thanks @grevil. We have to be careful then, because maybe the change will lead to issues where partial words are replaced, which might be undeserved. Maybe we'll need a setting, if we don''t find a good general solution. Thanks for finding out the reason so far!

🇩🇪Germany Anybody Porta Westfalica

MR !118 fixes the issue for me, now the menu item can be deleted as expected. Still the maintainer should check, if the implementation is the correct fix, I can't tell.

🇩🇪Germany Anybody Porta Westfalica

I can also confirm this issue. This entirely blocks deleting a custom menu item, as the error appears and the item can't be deleted. Implementation needs to be more defensive, as multiple people already ran into this.

🇩🇪Germany Anybody Porta Westfalica

Yeah several schema / config issues. Would be great if someone could proceed here. We're not actively using this module anymore, so won't invest much time unless paid FYI. Thanks!

🇩🇪Germany Anybody Porta Westfalica

anybody created an issue.

🇩🇪Germany Anybody Porta Westfalica

I think it would make sense to implement this in a submodule, because of the SDC Core module dependency.
Tests should then simply be INHERITED from the parent module, as the expected output should be THE SAME. That will be nice :)

🇩🇪Germany Anybody Porta Westfalica

Yeah feel free to propose it in a MR and we'll then see how good it works :)

🇩🇪Germany Anybody Porta Westfalica

Thanks, I'll ask @thomas.frobieter to give it a try in the affected project!

🇩🇪Germany Anybody Porta Westfalica

Confirming this major issue if aggregation is enabled.

🇩🇪Germany Anybody Porta Westfalica

@ressa feel free to implement this as MR to be reviewed. Happy to check it then! :)

🇩🇪Germany Anybody Porta Westfalica

@jaydarnell yes of course, sorry! Done!

🇩🇪Germany Anybody Porta Westfalica

Sorry just saw that this might be logically blocked by: 💬 Ckeditor5 compatibility Active

🇩🇪Germany Anybody Porta Westfalica

Any chance to merge this? D11 is out quite for a while and this module is widely used. Thank you!

🇩🇪Germany Anybody Porta Westfalica

@gausarts thanks! Enabling "use ajax" is also a nice workaround option, but I think it's just too dangerous. Slick views don't need AJAX logically, right and this happens with any slick view if admin_toolbar is used.

So any site maintainer changing the views setting would break it... From my perspective it needs to be fixed in code...

🇩🇪Germany Anybody Porta Westfalica

Unable to install modules: module 'zammad_webform_handler' is missing its dependency module token.

🇩🇪Germany Anybody Porta Westfalica

Nice!!
Confirming the element has been removed in 🐛 Captcha Math question needs to be more accessible Active

🇩🇪Germany Anybody Porta Westfalica

anybody created an issue.

🇩🇪Germany Anybody Porta Westfalica

Sorry needs to be reopened to fix the test, I didn't see that -.-

🇩🇪Germany Anybody Porta Westfalica

Drupal 7 is EOL.

🇩🇪Germany Anybody Porta Westfalica

Thanks!

🇩🇪Germany Anybody Porta Westfalica

Really nice @murat_kekic thank you!

Happy to merge this, once the comments are resolved and community RTBC'd it! :)

🇩🇪Germany Anybody Porta Westfalica

Gave it a short try (untested). Maybe someone would like to review it?

We're not using this module actively any more, so won't put much work into it ourselves.

🇩🇪Germany Anybody Porta Westfalica

Thanks @ericvl - I guess it's a bit different, but yes this needs further work! (In a follow-up).

.info.yml isn't D11 compatible and this issue only targeted the composer.json.

Follow-up: https://www.drupal.org/project/commerce_bulk/issues/3538293 📌 Drupal 11 and Commerce 3 compatibility Active

🇩🇪Germany Anybody Porta Westfalica

Hi @gausarts thanks for your quick reply!

I'l try debugging this deeper and will also try to reproduce this in vanilla Drupal using the admin menu modules, so hopefully you can also easily reproduce this. Very busy days, hopefully this week!

🇩🇪Germany Anybody Porta Westfalica

@kanthan would you mind to open a MR for the README.md to add that or even better add it here? 📌 Replace README.txt with README.md RTBC

🇩🇪Germany Anybody Porta Westfalica

Static patch from MR!22 current state attached

🇩🇪Germany Anybody Porta Westfalica

Definitely major, this breaks projects and updates. Any chance to tag a new release?

🇩🇪Germany Anybody Porta Westfalica

Prepared a hotfix on #6 for now. I'm really interested what causes this and how to properly fix it. Maybe it's the BigPipe/Dom/AJAX handling workarounds in Blazy?

Feel free to move this issue over to Blazy, if it belongs there. Thanks!

🇩🇪Germany Anybody Porta Westfalica

Okay, rewriting the attach() to once() works flawlessly:

    attach: function (context) {
      // _d.once(doSlick, _id, _element, context);
      once(_id, _element, context).forEach((mySlider) => {
        doSlick(mySlider);
      });
    },

So I guess this is really a blazy.once(.min).js bug?

🇩🇪Germany Anybody Porta Westfalica

I'm not totally sure if this is a bug in blazy.once or in slick. We've seen this on D11 projects, I'll try to find out, if same happens in D10.

🇩🇪Germany Anybody Porta Westfalica

Regarding #37 here's the related .min.js issue: https://github.com/kenwheeler/slick/issues/3649
Quite weird!

🇩🇪Germany Anybody Porta Westfalica

Any chance to merge this and tag a new release because of the "Major" status (broken projects)?
Thanks!

🇩🇪Germany Anybody Porta Westfalica

Thanks for the MR. Back to NR, would be nice if someone here could test it (or even better add a test)!

Code-wise this LGTM, still there might be an impact on performance, as each view is now executed twice? Do we have a way to improve that or are there better ways?
Otherwise I'd be fine to merge this after review. Thank you! :)

🇩🇪Germany Anybody Porta Westfalica

Please use a MR instead of patches. Any other ideas to solve this as clever as possible?

🇩🇪Germany Anybody Porta Westfalica

Thanks for your feedback @jurgenhaas. I agree the risks are low enough, the benefit is much higher. Let's merge this.

🇩🇪Germany Anybody Porta Westfalica

@mradcliffe thanks, yes I'm not against it in general, but I think you also understand that I'm struggling with changing the textfield to a textarea, as it's not "my" module, I just helped with maintainership... maybe we should wait for further maintainer feedback and until then you use the patch?

Mhmmmm

🇩🇪Germany Anybody Porta Westfalica

@lrwebks: Regarding removing the view when uninstalling the module, see https://www.drupal.org/node/2404447
enforced: is the key.

🇩🇪Germany Anybody Porta Westfalica

@lrwebks I guess we need to re-export the view: https://git.drupalcode.org/project/ad/-/blob/11.x/modules/ad_content_sch...

Guess something changed in the data structure or field names so that the view broke?

🇩🇪Germany Anybody Porta Westfalica

Perfect, you're right!

🇩🇪Germany Anybody Porta Westfalica

Perfect, thank you! I'll merge this, as it's a correct fix. If still an issue, please comment here @socialnicheguru!

🇩🇪Germany Anybody Porta Westfalica

@eduardo morales alberti @quicksketch, @fabsgugu maybe you'd like to take over maintainership?

🇩🇪Germany Anybody Porta Westfalica

Thanks, I guess the change will affect existing installations visually, adding a lot more space in many installations. Do we have other options?

🇩🇪Germany Anybody Porta Westfalica

Thanks, this also makes the captcha more accessible for bots, but I agree accessibility for humans is more important and in general this captcha type isn't highly secure.

RTBC! Let's merge this! :)

🇩🇪Germany Anybody Porta Westfalica

anybody made their first commit to this issue’s fork.

🇩🇪Germany Anybody Porta Westfalica

anybody made their first commit to this issue’s fork.

🇩🇪Germany Anybody Porta Westfalica

Thanks, good fix! Indeed in JS it should be lowercase!

🇩🇪Germany Anybody Porta Westfalica

A limited description field isn't a bug, it's a decision made by former maintainers, and I think it's not "wrong". It's more a specific need, I'd say. I'm not against it, but didn't see much community need for that in the past. Maybe you could just solve it custom using an alter hook?

🇩🇪Germany Anybody Porta Westfalica

anybody made their first commit to this issue’s fork.

🇩🇪Germany Anybody Porta Westfalica

anybody made their first commit to this issue’s fork.

🇩🇪Germany Anybody Porta Westfalica

I don't think so, furthermore this is specific to the various captcha types I think. Feel free to propose a MR, but I don't think this will make sense in all cases client-side. Better maybe implement per captcha type, where it makes sense?

🇩🇪Germany Anybody Porta Westfalica

@murat_kekic thank you! Could we maybe have a test that fails before these fixes and is green when fixed? That would ensure it's working as expected. Maybe as separate branch?

🇩🇪Germany Anybody Porta Westfalica

anybody made their first commit to this issue’s fork.

🇩🇪Germany Anybody Porta Westfalica

Thanks @jurgenhaas - how sure are we, that this doesn't introduce any unwanted side-effects? Generally I'd be fine with this change, but I'm not really sure about possible edge-cases, especially in existing installations.

🇩🇪Germany Anybody Porta Westfalica

anybody made their first commit to this issue’s fork.

🇩🇪Germany Anybody Porta Westfalica

Thanks @dhruv.mittal I left a comment. Which was the exact value for $id where this happened? Are we sure it's not a bug in hcaptcha?

🇩🇪Germany Anybody Porta Westfalica

anybody made their first commit to this issue’s fork.

🇩🇪Germany Anybody Porta Westfalica

Very nice work, thanks you!

🇩🇪Germany Anybody Porta Westfalica

@socialnicheguru if you desperately need 10.x compatibility, you could create a separate issue and change compatibility issues there, I'd be open to use the 10.x branch for a limited copy of 11.x that works with Drupal 10!

🇩🇪Germany Anybody Porta Westfalica

@lrwebks please use the separate branch: https://git.drupalcode.org/issue/ad-3535692/-/tree/3535692-remove-d10-co...
That's always a good idea to keep other peoples work and makes a lot of sense for such cases where the approach is totally different.

BTW. then 11.x module version also makes more sense... :D

🇩🇪Germany Anybody Porta Westfalica

@lrwebks and @socialnicheguru as 11.x is still in alpha, I'd vote for removing Drupal 10 compatibility in the module and all submodules and start with 11.x. Sooner or later we'll all land there and that's what alpha is for.

🇩🇪Germany Anybody Porta Westfalica

Can't we just look up, how the other link (it) formatters solve it? This isn't a dedicated feature for us, right?
So we should simply use the same logic or am I missing something?

🇩🇪Germany Anybody Porta Westfalica

Thanks! As of https://www.drupal.org/node/3450770 this should also work for all older Drupal versions. Merging!

🇩🇪Germany Anybody Porta Westfalica

I also have the gut feeling, that it's still happening under certain circumstances. Seems to happen more rarely, but after some time, certain translation strings still fall back to wrong default language. Nevertheless, I cannot rule out the possibility that contrib is involved.

🇩🇪Germany Anybody Porta Westfalica

No reply from @attisan so this seem stuck. I still think this should better be a contrib project or at least a submodule, as it's an edge-case and a bit risky.

🇩🇪Germany Anybody Porta Westfalica

@damienmckenna thanks a lot! I like this and I'd really like to merge this, but I'm not sure how many projects it might break, what do you think how risky this is? Doesn't seem to be fully bw-compatible in all edge cases?

Production build 0.71.5 2024