Account created on 20 May 2009, about 15 years ago
#

Merge Requests

Recent comments

πŸ‡ΊπŸ‡ΈUnited States gbirch

#6 works for me, fwiw.

πŸ‡ΊπŸ‡ΈUnited States gbirch

As noted in the issue description, I was unclear at the time of the report about the circumstances in which the passed user might be anonymous - I had the error, but not a lot of detail about the circumstances in which it was triggered. So I simply wrote a patch that tested for the existence of an array before calling a function that requires an array - pure defensive programming. But I now know that it can happen when using the Masquerade module and a user who has masqueraded now de-masquerades. Why this should be true I have not chased down, but honestly, this patch simply makes the code work when an anonymous user object is passed, which seems like good coding practice to me.

πŸ‡ΊπŸ‡ΈUnited States gbirch

@poker10
I have added the fix, a unicode test (easy), and a form validation test (harder - as there was no existing test for maxlength validation).

πŸ‡ΊπŸ‡ΈUnited States gbirch

As mentioned in my late comments on a closed issue ( https://www.drupal.org/project/drupal/issues/3270881 πŸ› [D7 PHP 8.1] strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in text_summary() Fixed ), it seems to me that the right, developer-friendly answer is to make drupal_strlen() behave as it does in PHP 7 when passed a NULL value - it should simply return 0.

I'm not sure what the right answer should be when handling other variable types: arrays, objects, scalar values. Casting to string can result in odd behavior - in particular the function will return 5 if given (string) $array, which just seems wrong and unhelpful. But perhaps that's a question for another day, as the vast majority of errors seem likely to be the result of passing NULL.

In short, can we just do https://www.drupal.org/project/drupal/issues/3270881#comment-15027276 πŸ› [D7 PHP 8.1] strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in text_summary() Fixed and call it a day? I'm happy to produce a patch or fork if that seems like the right answer. I'm not sure where a test should go - in UnicodeUnitTest?

πŸ‡ΊπŸ‡ΈUnited States gbirch

@maxilein: Wow, I see the problem - while Name fields on content bundles are listed, Name fields on user entities are not. Sorry, I'm out of ideas here.

πŸ‡ΊπŸ‡ΈUnited States gbirch

@maxilein, this configuration has to be done in the settings for the diff module at https://YOUR_SITE/admin/config/content/diff/fields If you do not start at that URL, you are starting in the wrong place. See the attached image of the top of that page.

The picture you have attached is for the field settings form, which, again, is not the right place. You need DIFF settings.

Hope that helps.

πŸ‡ΊπŸ‡ΈUnited States gbirch

See also https://www.drupal.org/project/recaptcha_v3/issues/3361927 ✨ Custom error logging Active for a more refined version of this approach.

πŸ‡ΊπŸ‡ΈUnited States gbirch

Note that the original patch is rolled against the alpha-5 tag. The MR is against -dev.

πŸ‡ΊπŸ‡ΈUnited States gbirch

gbirch β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States gbirch

@poker10, I don't think I agree with you about which code is "broken".

Prior to PHP 8, you could happily pass NULL to these functions in place of empty strings. The drupal_ functions you mention do not specify that the input must be a string in the PHP doc or otherwise. If they don't, then arguably the bug/incompatibility is in the drupal_ functions themselves, which don't behave as they used to, and not in the "broken" contrib code that uses them.

If you have been calling one of these functions in your custom/contrib code, how would you know that you must pass a string, if a) previously the functions accepted NULL values happily and b) now they don't, if the function signatures are unchanged? And from a purely practical point of view, what happens here is that the core code throws notices, and you don't immediately know what code called it without analyzing the back-trace. This is not a developer friendly result.

Finally, the whole POINT of those functions is to provide a wrapper around different PHP implementations, character sets, etc. This would be just one more compatibility wrapper.

πŸ‡ΊπŸ‡ΈUnited States gbirch

The patch should also check that the value is a string so as to prevent deprecation notices when NULL is passed to mb_strlen() by drupal_strlen().

πŸ‡ΊπŸ‡ΈUnited States gbirch

I understand that this precise issue is fixed, but the underlying problem with drupal_strlen() can be caused by many other things. Would it not be better for drupal_strlen() to return 0 if the $text is NULL?

I.e. add the following to drupal_strlen():

if (is_null($text)) {
  return 0;
}
πŸ‡ΊπŸ‡ΈUnited States gbirch

@maxilein The error from "NameFieldBuilder" is only going to come from a "name" field provided by the Name module (confusingly enough, the name of a user is not, in fact, a name field provided by the module). If you don't see a field listed on /admin/config/content/diff/fields that shows "Name" as its "Field Type," then are you sure you're really getting a NameFieldBuilder error? This is a field-level configuration problem, by the way - saving the user accounts (or any type of content) is never going to do anything for you. This is only solvable on the page mentioned.

πŸ‡ΊπŸ‡ΈUnited States gbirch

If you are getting the "Undefined array key "compare_format" in Drupal\name\Plugin\diff\Field\NameFieldBuilder->build()" warning, it appears that you can fix it by going to /admin/config/content/diff/fields and updating the configuration for any Name fields. Name does not provide a default diff format. Once you set one, the warning goes away.

Production build 0.69.0 2024