🇩🇪Germany @Anybody

Confirming we just had this error on the following path:
https://www.example.com/user/register?_wrapper_format=drupal_ajax&ajax_f...

Backtrace:

TypeError: mb_strlen(): Argument #1 ($string) must be of type string, array given in mb_strlen() (line 332 of /web/core/lib/Drupal/Core/Form/FormValidator.php).
#0 /web/core/lib/Drupal/Core/Form/FormValidator.php(332): mb_strlen()
#1 /web/core/lib/Drupal/Core/Form/FormValidator.php(246): Drupal\Core\Form\FormValidator->performRequiredValidation()
#2 /web/core/lib/Drupal/Core/Form/FormValidator.php(238): Drupal\Core\Form\FormValidator->doValidateForm()
#3 /web/core/lib/Drupal/Core/Form/FormValidator.php(238): Drupal\Core\Form\FormValidator->doValidateForm()
#4 /web/core/lib/Drupal/Core/Form/FormValidator.php(118): Drupal\Core\Form\FormValidator->doValidateForm()
#5 /web/core/lib/Drupal/Core/Form/FormBuilder.php(593): Drupal\Core\Form\FormValidator->validateForm()
#6 /web/core/lib/Drupal/Core/Form/FormBuilder.php(326): Drupal\Core\Form\FormBuilder->processForm()
#7 /web/core/lib/Drupal/Core/Controller/FormController.php(73): Drupal\Core\Form\FormBuilder->buildForm()
#8 /web/core/modules/layout_builder/src/Controller/LayoutBuilderHtmlEntityFormController.php(39): Drupal\Core\Controller\FormController->getContentResult()
#9 [internal function]: Drupal\layout_builder\Controller\LayoutBuilderHtmlEntityFormController->getContentResult()
#10 /web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array()
#11 /web/core/lib/Drupal/Core/Render/Renderer..php(638): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#12 /web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(121): Drupal\Core\Render\Renderer->executeInRenderContext()
#13 /web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext()
#14 /vendor/symfony/http-kernel/HttpKernel.php(181): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#15 /vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw()
#16 /web/core/lib/Drupal/Core/StackMiddleware/Session.php(53): Symfony\Component\HttpKernel\HttpKernel->handle()
#17 /web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle()
#18 /web/core/lib/Drupal/Core/StackMiddleware/ContentLength.php(28): Drupal\Core\StackMiddleware\KernelPreHandle->handle()
#19 /web/core/modules/big_pipe/src/StackMiddleware/ContentLength.php(32): Drupal\Core\StackMiddleware\ContentLength->handle()
#20 /web/core/modules/page_cache/src/StackMiddleware/PageCache.php(116): Drupal\big_pipe\StackMiddleware\ContentLength->handle()
#21 /web/core/modules/page_cache/src/StackMiddleware/PageCache.php(90): Drupal\page_cache\StackMiddleware\PageCache->pass()
#22 /web/core/modules/ban/src/BanMiddleware.php(50): Drupal\page_cache\StackMiddleware\PageCache->handle()
#23 /web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\ban\BanMiddleware->handle()
#24 /web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
#25 /web/core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php(36): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
#26 /web/modules/contrib/remove_http_headers/src/StackMiddleware/RemoveHttpHeadersMiddleware.php(49): Drupal\Core\StackMiddleware\AjaxPageState->handle()
#27 /web/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(51): Drupal\remove_http_headers\StackMiddleware\RemoveHttpHeadersMiddleware->handle()
#28 /web/core/lib/Drupal/Core/DrupalKernel.php(741): Drupal\Core\StackMiddleware\StackedHttpKernel->handle()
#29 /web/index.php(19): Drupal\Core\DrupalKernel->handle()
#30 {main}

No idea if it was a hacking attempt or what caused this, but the issue is real.

Just wondered, why GA (via google_tag) was not blocked on an older project using klaro and the reason was, that this update was missng.

Maybe Klaro should add a mechanism to update the default configs to ensure the relevant entries are up to date? (And better they exist than they do not?)

TL;DR: Voting for update hooks in such cases to add the relevant / new entries, if not yet existing. Should we open a follow-up for that?

After adding the new entries, Klaro blocked correctly, but this is a bit dangerous for "normal" users?

@maintainers: Any chance to merge this finally?

See: https://git.drupalcode.org/project/klaro/-/blob/3.x/README.md

For Google Tag 2.x you may consider to disable the noscript snippet (see #3106318).

Thanks for the super helpful and important investigations @grevil!

I think we should do two things:
a) handle the different link types (rel's: (information_link, payer-action, redirect, ...?) as described in 🐛 Needs to redirect users on UNPROCESSABLE_ENTITY > PAYER_ACTION_REQUIRED - No checkout possible Active in ✨ Print details from error messages returned by paypal Active so that if user action is needed, it works!
b) Try to reduce the cases where a user action is needed by default, e.g. might mean to include a default shipping (country) rate in the first call in the cart

Thanks @grevil. I'm not sure if we should track the point in time, when the order was paid in full as an additional base field or instead should calculate it based on the tracked payments (IF $order->isPaid() is true).

Let's wait for maintainers feedback on how they'd say it's best.

Which way ever we go, we need to ensure that the information is always consistent, for example if a payment was refunded afterwards.

anybody → created an issue.

Thanks @safetypin! Google found this for me: https://drupal.stackexchange.com/questions/55279/how-can-i-set-the-langu...

Great work for a great feature based on the work done by @vmarchuk! Thanks for the various important finding and fixes!

@vmarchuk will you or your team have a look soon so we don't lose focus here? Would be wonderful to have this feature released soon IMHO!

Works as expected in my testing! RTBC!

Here we go, sorry for the code style fixes in the same commit. -.-

anybody → created an issue.

@grevil: I think the logical mistake is, that only product line items should be included in the JSON generally and never adjustments. I didn't look into the code, but hopefully it's easy to find the reason how VAT comes in here.

Thank you so much @jsacksick and @vmarchuk! We're on it!

And let's please close all fixed subissues then.

Guess this makes sense as next step, but we should first ensure we have a valid concept.

@derekw is this maybe fixed in the meta issue already? 📌 [META] 11.x feature updates Active
Could you priovide a MR otherwise perhaps?

@grevil this is ready so far, please review by time and autofix issues. I think we'll surely find some points for @lrwebks left to resolve next week.

Let's do this as follow-up. @lrwebks has a list of possible Scheduler issues that might help with the implementation.

alexpott → credited anybody → .

+1 thank you @grevil!

@tomtech any chance to merge this? Would be helpful and is a nice improvement I think?

@vmarchuk the testing environment is currently broken, could you revive it?
TypeError: Drupal\Core\Entity\Sql\DefaultTableMapping::Drupal\Core\Entity\Sql\{closure}(): Argument #1 ($definition) must be of type Drupal\Core\Field\FieldStorageDefinitionInterface, __PHP_Incomplete_Class given in Drupal\Core\Entity\Sql\DefaultTableMapping::Drupal\Core\Entity\Sql\{closure}() (line 175 of core/lib/Drupal/Core/Entity/Sql/DefaultTableMapping.php).

So we could proceed the comparison. Would it be possible to share a screenshot of your Drupal Stripe payment gateway settings? (admin/commerce/config/payment-gateways/manage/stripe_payment_element)

@tomtech plans to merge this into 2.x? As paypal is quite common...

Sorry, this wasn't the reason for our trouble. I had to use single_use for expected payment methods to appear.

@damienmckenna thanks for your reply. In my eyes both is true.

Currently the issue is, that theres a "Live" vs. "Sandbox" switch, but you can only store public / private key for one of them. This combination doesn't make much sense in our eyes.
When switching the radio, you have to exchange the keys and enter the opposite ones and when switching back, same again...

The goal here is to add separate fields & config values for live and sandbox credentials, so you can switch at any time, if you need to, by just switching the radio.

Of course it should also be documented how to set these values in settings.php.

Yeah, I had similar thoughts.

I like:

• PayPalErrorHandler
• PayPalErrorResponseHandler
• PaymentExceptionHandler
• PaymentErrorHandler

PayPalErrorHandler or PayPalErrorResponseHandler are most clear and descriptive IMHO.

Anyway the service gives us wonderful options for future extensions and changes. Nice work @Grevil! :)

Okay I think we should first and finally resolve 🐛 Documentation improvements Active to improve documentation and add details from the reply here. Maybe close this as duplicate afterwards?

anybody → created an issue.

Sorry but I can't reproduce it. Any chance that there's a misconfiguration? Maybe wrong text format for the affected contents, where Glossify isn't enabled or configured wrongly?

@jsacksick sadly no, I think there are cases where Paypal asks for further verifications, see the linked issue:

Using PayPal Checkout with the buttons in cart can sometimes lead to a PayPal error with
name: UNPROCESSABLE_ENTITY
details: issue: PAYER_ACTION_REQUIRED

So PAYER_ACTION_REQUIRED is one of the possibly more complex examples. Refunds might be a simpler one.

An error handler service or helper would make sense if we were doing some mapping between a code and a human readable message which isn't the case here? We're just spitting the error returned right?

Nope, I think we need to do exactly that - handle messages differently based on their type and eventually perform additional actions, so there's some complexity.

For example, see here: 🐛 Needs to redirect users on UNPROCESSABLE_ENTITY > PAYER_ACTION_REQUIRED - No checkout possible Active Paypal expects to (re)direct the user to a certain action URL.

@jsacksick thanks - yes I also think we should use the messenger for such details. What do you think about the general architecture - putting this into a dedicated service to handle the different types of messages etc.?

Just my 2 cents: I think handling the JSON returned from PayPal is essential, especially for cases, where buyer interaction is needed, like in 🐛 Needs to redirect users on UNPROCESSABLE_ENTITY > PAYER_ACTION_REQUIRED - No checkout possible Active .

Putting the handling into a dedicated service also makes a lot of sense to me, to being able to centrally handle such messages and related actions. For example, showing the informational link or even redirecting users. Otherwise, such logics may mess up the code around.

Perfect!

Simple, but effective fix! :) Thanks!

Thanks, yes should be investigated and fixed, please.

I can confirm this. Input should definitely always be trimmed for this reason. As it causes a hard exception, I'd say this is high prio.

No idea yet, sorry. Sounds like some kind of race condition / timing issue...

Please provide a proper description by filling the issue template:

I can confirm this issue, a client entered the YouTube video URL without www. and that lead to oEmbed error message "No matching provider found."

🐛 Remove YouTube videos in library cause "No matching provider found." Active

Suggestion implemented in MR, please review. Is it fine to place the variable in the Exception or do we need any kind of escaping? (I don't think so). Without these details, the message will not be helpful to show in any way (in my opinion).

Here is the implementation and its tests: https://git.drupalcode.org/search?search=No%20matching%20provider%20foun...

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

3.x is unsupported, what's the situation in 5.x? Can we close this outdated?

anybody → created an issue.

@rkoller thanks, yes that's because this condition plugin doesn't seem to be applied on blocks, if I remember correctly.
We ran into this in contrib modules that allow all condition plugins, e.g. https://www.drupal.org/project/posthog → or https://www.drupal.org/project/context → - unsure if there's also a place in core where it's shown.

@Grevil can you please update the IS accordingly by time, if I'm right?

Perfect, thanks @grevil! :)

Yeah maybe one day... if anyone does it. We won't implement this ourselves.

@thomas.frobieter core lazy loading is already supported in photoswipe now. Using the lazy module is quite untypical since lazy has native support, but if anyone is interested to write that as contrib module, we're happy to link that.

Thanks @grevil totally makes sense and code LGTM so far. Sadly, I have no clue, how an update hook for a condition plugin can be written. I'm also unaware of any existing examples, while I think they possibly exist.

Is there maybe any core maintainer or someone else experienced with that? We'd really like to finish this, for UX and bugfixing reasons, as currently it's kind of broken by design ;)

LGTM, just one question for @thomas.frobieter - and ready for his testing and CSS implementation.

Whao this is crazy good @grevil! Thank you! LGTM, let's wait for @grimreaper's feedback!

Nice, makes sense!

Ok here are the new settings as discussed with @thomas.frobieter, to implement as checkboxes.

Hide controls [hide]
Show vidstack controls [show_vidstack]
Show vidstack controls (on hover) [show_vidstack_hover]
Show native controls [show_native]

From my opiniton this would be a perfect example for a settings.php setting in core which can be overridden... but let's see what the others think.

@grevil thanks, that looks exactly as planned. Regrading the entity question: I thinkt here must be a way to get all entity types / bundles that use a certain field type, here: entity_access_password_password

Those should then be processed, I think. Do you agree?

Rerolled against latest dev!

Yeah in general this is just my 2 cents. We should not be too strict with this, it won't harm too much if it's not perfect yet.

Thanks for the detailed feedback @tomtech. Let me add some further points:
1. I indeed think that the current "Items have been added to the cart"-message is confusing on login and unexpected, meaning no good UX for the user
2. I agree that a clearer message would be helpful to let the user know that carts have been combined - but definitely shouldn't be the same as now. With a clear message, it would be a bonus, while the current message is a malus.
3. I agree that creating the message might better fit into commerce_combine_carts then, but that also means that the current message from commerce_add_to_cart_confirmation needs to be suppressed in that case, so probably it's an issue in both modules in combination then?

To sum things up from my perspective:

• commerce_add_to_cart_confirmation
• Needs more information about the context from where items have been added to cart
• Should not show the confirmation in all of these contexts

commerce_combine_carts

• Needs to provide one of these contexts
• Might show a different message, informing the users that carts have been merged on login. That could also be a regular Drupal "Success" message

So from my perspective, it looks like most of the essential things still need to be resolved in commerce_add_to_cart_confirmation code-wise and logically?

EOL

Great work! All green now, let's merge this.

@droath I now opted the module into security advisory policy. I'll wait before committing other fixes to this module to hopefully get in contact with you. Anyway, I'll always respect your work as original maintainer and hope we can help you maintaining the module.

@grevil thanks, LGTM, let's wait for further feedback.

LGTM and resolves the error

@lazzyvn no, this is not specific to admin theme, it's specific to paragraphs_table. We can't assume other containers should be horizontally scrollable.

I understand your special relationship with bootstrap 5 theme, but this in a very helpful module for anyone, also Gin users and other Drupal users. Please consisder that. And currently this is neither working for Gin nor for Claro, without implementing this. People should never be forced to use your bootstrap 5 theme to have an acceptable UX. In general modules should not rely on specific admin themes.

Yeah big +1 on this. Currently a wide table kills the form displays. Definitely the wrapper should be scrollable. We should ask @thomas.frobieter for a MR.

Thanks! A new release would be great!

Thanks @thomas.frobieter great work! I think that's totally fine. RTBC from my side! :) Really nice improvement, we should suggest this for core link fields widgets as follow-up.

Sorry, stupid me... wrong project! Should have been on https://drupal-mrn.dev/

Thanks, merged! :)

anybody → created an issue.

I'm now tagging a new release.

Merge train started in https://git.drupalcode.org/project/billwerk_subscriptions/-/merge_reques...

Thanks @grevil! Yes I think we should unset it, at least in the UI. In the backend we could eventually map it...

@lrwebks: Ok let's do it like this, as just discussed with @thomas.frobieter:

Replace the current controls checkbox with a "Controls" select with the following options:
- Hide controls
- Show native controls
- Show vidstack controls

()Maybe in the future we'll have further ones like "Show XX controls on hover"...

This needs no update hook, as the module is not used anywhere yet.

We just got feedback from Billwerk that this bug is now fixed on their side. I tried with removed workaround and everything seems to work well now, so I'll remove the workaround then.

Before implementing anything here, I think we need to come to a consensus what's expected and what should be handled (in the UI) or if this is essentially a vidstack bug.

I think when using Vidstack, we always want to use the Vidstack controls and I'd expect Vidstack to hide the native controls. Adding a further checkbox would delegate this confusion to the user, so I'd vote to either report this as bug to Vidstack and see what they say or (at least for now) set the attributes in a way that isn't confusing, so the user only controls the vidstack controls and if they are OFF, other controls are also hidden.

Yes, I'd also prefer A as a nice improvement. B could be an additional option using a select like we for example know from field groups? Maybe as follow-up?

When solving this, either the formatter should get a "Use widget setting / default" option or the widget should generally override the formatter setting and the formatter should have a description informing about that like "Notice: If the widget allows overriding, this setting will have no effect."

I agree, but as nobody complained so far, we should wait for a MR to review, if someone needs it.

Thanks for the fix on 🐛 Micon missing schema for its fields definitions Fixed ! RTBC!

Nice work @Grevil. Some final additions and please check code-style. The complaints look weird to me, but we should ensure code style is fine! :)

Nice finding @jan kellermann that looks promising and totally makes sense! Thank you for the super quick reply!

anybody → created an issue.

@thomas.frobieter I just had the idea to contact @chi, maintainer of https://www.drupal.org/project/twig_tweak → and ask if we could maybe implement a merging solution that respects a certain field display formatter for drupal_field(). What do you think about that?