#plain_text doesn't render empty-like values (e.g. 0 and "0")

Thank you for sharing the issue. I tested and confirmed that this bug exists.

This seems to be caused as Drupal\Core\Render\Renderer::doRender() doesn't check the value of #plain_text properly.

    // All render elements support #markup and #plain_text.
    if (!empty($elements['#markup']) || !empty($elements['#plain_text'])) {
      $elements = $this->ensureMarkupIsSafe($elements);
    }

Here empty() is not proper and we should use isset() instead, I think.

I created a patch. I thought it's better to add tests for the following cases and added such tests, too.

$render_arrays = array(
  array('#markup' => "0"),
  array('#markup' => 0)
);

I'd like someone to review this. Thank you.

This patch and test make complete sense to me. I just ran into this issue while working on #2922896: Write dedicated rendering tests. I am rerunning the tests and then I am going mark this RTBC. Hopefully, it will get committed or someone will ask for more test coverage.

Today I ran into this issue as well.

So, here a rebased version of the patch.

It seems that the patch needs to fix some coding standard problems.

I'm going to add the novice tag in case someone wants to do it.

Changes addressed in comment #13 & also added an interdiff.

Looks great, thanks!

Nice find. I think we should try to keep the #markup and #plain_text logic as similar as possible.

1. +++ b/core/lib/Drupal/Core/Render/Renderer.php
   @@ -380,7 +380,7 @@ protected function doRender(&$elements, $is_root_call = FALSE) {
   -    if (!empty($elements['#markup']) || !empty($elements['#plain_text'])) {
   +    if (!empty($elements['#markup']) || isset($elements['#plain_text'])) {
   

   Let's change both !empty()<code> to <code>isset()

2. +++ b/core/lib/Drupal/Core/Render/Renderer.php
   @@ -744,11 +744,11 @@ protected function xssFilterAdminIfUnsafe($string) {
   -    if (empty($elements['#markup']) && empty($elements['#plain_text'])) {
   +    if (empty($elements['#markup']) && !isset($elements['#plain_text'])) {
          return $elements;
        }
   

   This whole check seems pointless.

3. Then the whole function becomes:

     protected function ensureMarkupIsSafe(array $elements) {
       if (isset($elements['#plain_text'])) {
         $elements['#markup'] = Markup::create(Html::escape($elements['#plain_text']));
       }
       elseif (!($elements['#markup'] instanceof MarkupInterface)) {
         // The default behaviour is to XSS filter using the admin tag list.
         $tags = isset($elements['#allowed_tags']) ? $elements['#allowed_tags'] : Xss::getAdminTagList();
         $elements['#markup'] = Markup::create(Xss::filter($elements['#markup'], $tags));
       }
   
       return $elements;
     }
   

   Note that we don't need to check the isset($elements['#markup'] in the second condition because it must be set to have got here. I think reviewing this code we should make this fix but also open a follow-up issue to inline ensureMarkupIsSafe into the main render function because it is not needed as a separate function. It just makes it most complex.

   If we inlined the function it would look like:

       if (isset($elements['#plain_text'])) {
         $elements['#markup'] = Markup::create(Html::escape($elements['#plain_text']));
       }
       elseif (isset($elements['#markup'] && !($elements['#markup'] instanceof MarkupInterface)) {
         // The default behaviour is to XSS filter using the admin tag list.
         $tags = isset($elements['#allowed_tags']) ? $elements['#allowed_tags'] : Xss::getAdminTagList();
         $elements['#markup'] = Markup::create(Xss::filter($elements['#markup'], $tags));
       }
   

   which in my opinion is easier to grok than having the misdirection of the method call inside the if(). But that is all followup material.

4. I think we need to expand the test coverage to ensure sensible things happen with nulls and empty strings

Attached a patch that addresses remarks 1, 2 and 4. I've added a check for '' and NULL to the tests. Also attached a patch containing only the test and an interdiff.

Looks great.

Thanks!

Crediting @weboide for discovering and filing the issue. Crediting myself for a review that directly influenced the patch.

Committed and pushed 018bd7c5e2 to 8.6.x and 1675b7b81c to 8.5.x. Thanks!