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.
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.
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!
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.
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.
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!
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 :)
Yeah feel free to propose it in a MR and we'll then see how good it works :)
Thanks, I'll ask @thomas.frobieter to give it a try in the affected project!
Confirming this major issue if aggregation is enabled.
@ressa feel free to implement this as MR to be reviewed. Happy to check it then! :)
@jaydarnell yes of course, sorry! Done!
Thanks!
Sorry just saw that this might be logically blocked by: 💬 Ckeditor5 compatibility Active
Any chance to merge this? D11 is out quite for a while and this module is widely used. Thank you!
@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...
Unable to install modules: module 'zammad_webform_handler' is missing its dependency module token.
Nice!!
Confirming the element has been removed in
🐛
Captcha Math question needs to be more accessible
Active
Sorry needs to be reopened to fix the test, I didn't see that -.-
Drupal 7 is EOL.
Thanks!
Really nice @murat_kekic thank you!
Happy to merge this, once the comments are resolved and community RTBC'd it! :)
anybody → created an issue. See original summary → .
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.
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
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!
@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
Static patch from MR!22 current state attached
Definitely major, this breaks projects and updates. Any chance to tag a new release?
Hotfix MR!15 as static patch attached.
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!
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?
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.
anybody → created an issue.
Regarding #37 here's the related .min.js issue: https://github.com/kenwheeler/slick/issues/3649
Quite weird!
Any chance to merge this and tag a new release because of the "Major" status (broken projects)?
Thanks!
Thanks, merged!
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! :)
Please use a MR instead of patches. Any other ideas to solve this as clever as possible?
Thanks for your feedback @jurgenhaas. I agree the risks are low enough, the benefit is much higher. Let's merge this.
@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
@lrwebks: Regarding removing the view when uninstalling the module, see
https://www.drupal.org/node/2404447 →
enforced:
is the key.
@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?
Perfect, you're right!
Perfect, thank you! I'll merge this, as it's a correct fix. If still an issue, please comment here @socialnicheguru!
@eduardo morales alberti @quicksketch, @fabsgugu maybe you'd like to take over maintainership?
Thanks, I guess the change will affect existing installations visually, adding a lot more space in many installations. Do we have other options?
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! :)
Thanks, good fix! Indeed in JS it should be lowercase!
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?
anybody → made their first commit to this issue’s fork.
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?
@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?
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.
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?
Very nice work, thanks you!
@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!
@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
@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.
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?
Thanks! As of https://www.drupal.org/node/3450770 → this should also work for all older Drupal versions. Merging!
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.
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.
@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?