Title in template_preprocess_item_list() is typecasted as string

Thanks for creating the issue. Tagging as Novice and tweaking some wording.

Removed string typecasting from title in attached patch.

I think we need something like $variables += array('title' => ''); but not useless redefinition.

Miroshnik, I completely agree with you and I would remove that extra typecasting code from next patch. Could you please elaborate this issue for me in detail. I am digging into code but not finding the exact scenario to reproduce this issue.

We're working Bootstrap theme porting to D8. It has enchainment for item_list theme that allow to override list title output and provide renderable array instead of pure string. E.g. we won't to use

instead of recently hardcoded

. (string) $variables['title'] in template_preprocess prevent it. As this typecasting doesn't add anything, doesn't prevent anything from error or notice but breaks original preprocess flow it has to be removed.

Thanks for working on this @er.pushpinderrana!

The line from preprocess can be removed, and the default item-list.html.twig should have 'if title' changed to 'if title is not empty'.

Thankyou Miroshnik and Cottser for explaining this issue in detail.

I have made the required changes in files and also tested its impact by calling item_list in my custom module. I am sharing some code with you to confirm my approach towards fixing this issue.

theme.inc: Removed this unused code $variables['title'] = (string) $variables['title'];

item-list.html.twig: Replaced {%- if title -%} with {%- if title is not empty -%}.

I have tested this function by creating module and using '#theme' => 'item_list', for rendering output.

$items = array('Test One', 'Test B', 'Test C', 'Test D');
    return array(
      '#theme' => 'item_list',
      '#items' => $items,
      '#title' => '' // Test Title Empty Issue
    );

I have noticed one thing when I was testing this issue found {%- if title -%} also works fine and take care of empty value. I mean if title is empty then <h3></h3> don't render on page. But when I gone through twig template engine documentation I found {%- if title is not empty -%} is there to validate not empty condition. So in this patch I am using this code but still I am unable to reproduce any case in which {%- if title -%} code fail. If you have any scenario for this please share with me.

Well, it should work with such values:

• '#title' => 'text', // output
  

  text

• '#title' => '', // do not output anything
• '#title' => NULL, // do not output anything

and allow handle #title in custom hook_preprocess_item_list() as I want.
In our particular case we're passing '#title' => array('title' => 'TEXT', 'attributes' => ...),

However patch looks fine.

Hmm, if we can avoid adding the 'is not empty' we probably should. This should also have some automated test coverage as well, that should help decide what solution to go with here.

Cottser I have checked both code for number of cases and found both work fine except following case.

'#title' => 0 if we use {%- if title is not empty -%} it render title value <h3>0</h3>but {%- if title -%} print nothing.

'#title' => '0' if we use {%- if title is not empty -%} it print title value <h3>0</h3> but {%- if title -%} print nothing.

In this case we pass '0' value in title then {%- if title -%} don't render title because it treat this as FALSE condition but <code>{%- if title is not empty -%} treat this as TRUE condition. I am sure no one like to make title '0' of any page so we can go ahead with {%- if title -%}. In this patch I am using {%- if title -%} instead of {%- if title is not empty -%}.

PS: I also tried both code by passing FALSE in title and both works fine. Don't render <h3>FALSE</h3>.

I forgot about the pesky '0' case. So for this one, 'is not empty' for sure.

I'm not sure about a blanket 'is not empty' but I agree it would make sense to evaluate conditional logic in templates. Not really anything new though, this issue would still exist if we were still on PHPTemplate.

In #7 patch, I have use 'is not empty' and its working fine. I have tested this patch for most of cases, so we can go ahead with this patch.

drupal8-title-typecasted-2260059-7.patch

Yep #7 looks good. This needs automated test coverage:

https://drupal.org/contributor-tasks/write-tests

Cottser, I need your support on this. As I'm digging into drupal 8 test cases, found testItemList() is already there. When I ran this test case, it executed successfully and produces following message.

Do I need to write another test case here or need to edit this test case itself. Please suggest.

We would want to add a test case/test code with a title of "0" and make sure the 0 is output.

I'm just updating the summary of this issue.

Rerolled for PSR-4. No other changes.

I've added one more test for the original issue summary bug:

Typecasting this prevents themes that wish to pass renderable arrays as the title.

Also think this one is RTBC thanks for finding the bug and fixing this.

Additional tests address concerns identified in the issue summary and in comments. Looks good.

Committed ad64edd and pushed to 8.0.x. Thanks!