Problem/Motivation

\Drupal\config_translation\FormElement\Textarea can cause deprecations on PHP 8.1 because it passes NULL to a string function.

Steps to reproduce

Run \Drupal\Tests\node\Functional\NodeTypeTranslationTest on PHP 8.1

Proposed resolution

Fix code to not pass NULL to a string function.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

Status: Active » Needs review
FileSize
1.15 KB

Here's the fix from the meta.

andypost’s picture

Without patch

There was 1 error:

1) Drupal\Tests\node\Functional\NodeTypeTranslationTest::testNodeTypeTranslation
Exception: Deprecated function: str_word_count(): Passing null to parameter #1 ($string) of type string is deprecated
Drupal\config_translation\FormElement\Textarea->getTranslationElement()() (Line: 17)

...
ERRORS!
Tests: 2, Assertions: 30, Errors: 1.

Unsilenced deprecation notices (69)

  69x: strtolower(): Passing null to parameter #1 ($string) of type string is deprecated
    59x in NodeTypeTranslationTest::testNodeTypeTitleLabelTranslation from Drupal\Tests\node\Functional
    10x in NodeTypeTranslationTest::testNodeTypeTranslation from Drupal\Tests\node\Functional

with patch test pass

OK (2 tests, 35 assertions)

Unsilenced deprecation notices (93)

  93x: strtolower(): Passing null to parameter #1 ($string) of type string is deprecated
    59x in NodeTypeTranslationTest::testNodeTypeTitleLabelTranslation from Drupal\Tests\node\Functional
    34x in NodeTypeTranslationTest::testNodeTypeTranslation from Drupal\Tests\node\Functional

not clear why second log has more failures of strtolower()

alexpott’s picture

@andypost because it gets further in the test. The point of listing the test is not say the test will be green after this change on PHP 8.1 the point it to say what triggers the deprecation so we can investigate whether this is the best fix and why at this point $translation_config is NULL.

daffie’s picture

Status: Needs review » Needs work
+++ b/core/modules/config_translation/src/FormElement/Textarea.php
@@ -14,13 +14,15 @@ class Textarea extends FormElementBase {
+    if ($translation_config !== NULL) {

To me this is not the right check. The PHP functions str_word_count() and substr_count() both want the first parameter to be a string value. See: https://www.php.net/manual/en/function.str-word-count.php and https://www.php.net/manual/en/function.substr-count.php.

    if (is_string($translation_config)) {
alexpott’s picture

#5 is pretty moot. We map config that has the type 'text' to '\Drupal\config_translation\FormElement\Textarea' - so $translation_config can only be NULL or not set a string.

I guess there is a question about whether or not we should cast the value to something that complies with the config schema - however then we'd have to deal with nullables so back to the same issue.

Another question that comes to mind is should we use the source config if the translation config is NULL as a guide to the number of rows.

[Edit: correctness]

alexpott’s picture

Status: Needs work » Needs review
FileSize
825 bytes
1.15 KB
andypost’s picture

should we use the source config if the translation config is NULL as a guide to the number of rows.

I see no reason in it

daffie’s picture

Status: Needs review » Reviewed & tested by the community

As the variable $rows_newlines has a minimum value of 1, the variable $rows also has a minimum value of 1. Changing the default value of the variable $rows to 1 is to me the right solution.
It looks good to me.

  • catch committed 2145374 on 9.3.x
    Issue #3240174 by alexpott, andypost, daffie: \Drupal\config_translation...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed 2145374 and pushed to 9.3.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.