Problem/Motivation

If a blank string is passed to ProcessedText::preRenderText() there's no reason to run any of the logic processing it, is there?

Steps to reproduce

Pass a blank string through ProcessedText::preRenderText()

Proposed resolution

Return early in ProcessedText::preRenderText() if $element['#text'] is a blank string.

Remaining tasks

Provide a MR.
Add test coverage.

User interface changes

n/a

Introduced terminology

n/a

API changes

TBD

Data model changes

n/a

Release notes snippet

TBD

Issue fork drupal-3554602

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

damienmckenna created an issue. See original summary.

damienmckenna opened merge request !13589

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Needs review

The MR simply returns early if the string is empty.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Related issues: +#3294680: PHP8.1 deprecation: str_replace(): Passing null to parameter #3

Related: #3294680: PHP8.1 deprecation: str_replace(): Passing null to parameter #3

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Possible to get a simple unit test around this

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

oily’s picture

oily commented

Added a unit test. Needs work.

harivansh’s picture

harivansh commented
Assigned: Unassigned » harivansh
harivansh’s picture

harivansh commented
Assigned: harivansh » Unassigned
Status: Needs work » Needs review
oily’s picture

oily commented

Now the test-only test output is:

There was 1 error:
1) Drupal\Tests\filter\Unit\ProcessedTextTest::testPreRenderText with data set "empty text" (['', null, '', []], ['', null, '', [], ''])
Drupal\Core\DependencyInjection\ContainerNotInitializedException: \Drupal::$container is not initialized yet. \Drupal::setContainer() must be called with a real container.
/builds/issue/drupal-3554602/core/lib/Drupal.php:178
/builds/issue/drupal-3554602/core/lib/Drupal.php:439
/builds/issue/drupal-3554602/core/modules/filter/src/Element/ProcessedText.php:163
/builds/issue/drupal-3554602/core/modules/filter/src/Element/ProcessedText.php:72
/builds/issue/drupal-3554602/core/modules/filter/tests/src/Unit/ProcessedTextTest.php:48
ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

Not sure if that error should be fixed? And a test failure should replace it..

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: -Needs tests +Needs issue summary update

Looking at the IS and MR appears to be doing more then described. If they're needed those other checks don't appear to be captured in tests.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.