Prestashop: Regression on translations with multiple includes of partials template on FO

[202-ecommerce]

Describe the bug

We found a bug regression explain on this commit comment.

https://github.com/PrestaShop/PrestaShop/commit/f91ef20e86308ff7e6087f5e5130d44c0a8ff842#r34174985

To Reproduce

You nee to have a theme with a template

• registration.tpl

{extends file="module:mymodule/views/templates/front/page.tpl"}
{block name='left_content'}
<div>
  <h1 class="h4 text-center mb-4">{l s='Inscription' mod='mymodule'}</h1>
  <div class="registration-form">
    {include file='module:mymodule/views/templates/front/_partials/credentials-form.tpl'}
  </div>
</div>
{/block}

• mymodule/views/templates/front/_partials/credentials-form.tpl

```smarty
<div class="credentials-form">
  {block name='notifications'}
    {include file='_partials/notifications.tpl'}
  {/block}

    <form action="{$actionLink}" method="post">

        <section>
          {block "form_field"}
            <!-- First name -->
            <div class="form-group">
              <label for="credentials-firstname" class="form-control-label">
                {l s='First name' mod='mymodule'}
              </label>
          </div>
        </section>
....

{l s='First name' mod='mymodule'} will not be transalated. The key of the translated string is not found.

We found the bug in config/smartyfront.config.inc.php on a commit... It looks like a regression.

In case of multiple includes of templates on the theme, $smarty->source->name value of the second template return the first value (in our sample, 'fisrtname' is associated to notification.tpl instead of credentials-form.tpl as translation manager generated it). After that, translation of the second template are not found.
We test to reintroduce $filename and older code it works like a charm !

Additionnal information
PrestaShop version: 1.7.4.x to develop
PHP version: N/A

[PierreRambaud]

Hi and thanks for the explanation.

I'm not able to reproduce:

I edit my theme and include:

  <div class="registration-form">
    {include file='module:ps_facetedsearch/views/templates/front/catalog/test.tpl'}
    {include file='module:ps_facetedsearch/views/templates/front/catalog/test2.tpl'}
  </div>

then create both file with:

{l s='First name' mod='ps_facetedsearch'}
{l s='First name test' mod='ps_facetedsearch'}
{l s='First name blyat' mod='ps_facetedsearch'}

And in my BO I have:

I'm testing on the latest 1.7.6, did I do something wrong? :thinking:

[202-ecommerce]

First, template test2 need be included on test.tpl

Translations are not same on all template, put some translated string after included test2.tpl on test.tpl. You can add other translations on test2.tpl.

On your backoffice, it will be ok, you can change your translation.

But on frontoffice, translations on test2.tpl will be ok but test.tpl will be ko.
On your php translation file fr.php, you will have the right syntax:

$_MODULE['<{mymodule}prestashop>credentials-form_20db0bfeecd8fe60533206a2b5e9891a'] = 'Prénom';

but if you change the key (for us '<{mymodule}prestashop>credentials-form_' by '<{mymodule}prestashop>notifications_') from 'test' by 'test2' your translations will be OK on your frontoffice. That's because the name of the template is 'overrided' by the inclusion of the partial subtemplate.

[202-ecommerce]

@PierreRambaud We found today that in some case we need $smarty->source->name and in other case with included $smarty->template_resource to return the good current template name.

We will be back with new investigation in a few moment...

[PierreRambaud]

@202-ecommerce Really thanks for your feedback, I was completly stuck ^^'
I'm still trying to reproduce.

[202-ecommerce]

@PierreRambaud

Here you can find a sample module to see the bug. I hope helpfull !

issue.zip >>> Edit: do not consider this version

[202-ecommerce]

Sorry, little mistake on the previous module

issue.zip

[khouloudbelguith]

Hi @202-ecommerce,

I did not manage to reproduce the issue.
I use this theme including your module.
mythem.zip
https://drive.google.com/file/d/1cSouX1FUpqJtk5r_8--IgW5NENtALfix/view
Thanks to check & feedback.

[202-ecommerce]

The bug is not in backoffice but in front office.

I forget to send you a frontoffice controller URL to test:
http://your-domain.com/module/issue/custom

[khouloudbelguith]

@202-ecommerce, thanks for these clarifications.
I manage to reproduce the issue also with PS1.7.6.0rc1.

I’ll add this to the debug roadmap so that it’s fixed. If you have already fixed it on your end or if you think you can do it, please do send us a pull request!
Thanks!

[PierreRambaud]

Same, and i'm working on it to find a solution as fast as possible.

[202-ecommerce]

Not fixed for us for now.
We look for the right condition on config/smartyfront.config.inc.php between $smarty->source->name and $smarty->template_resource. It's a little tricky to find why the nested template return child template name after calling it. Perhaps a Smarty bug, but not sure...

[PierreRambaud]

Sam with $smarty->inheritance, the tplIndex still indicates the wrong index :thinking:

[PierreRambaud]

I think it's a Smarty bug, but I can't confirm, the selected source template is not the good one :disappointed:

[202-ecommerce]

This code looks like work, but don't know exactly why...
It look like there is a problem only when we have a mix of resources in this case : "file:" (templates from theme) and "module:" (templates files from modules)

//config/smartyfront.config.inc.php:220
    $resourcePrefix = substr($smarty->source->resource, 0, strpos($smarty->source->resource, ':'));
    $templatePrefix = substr($smarty->template_resource, 0, strpos($smarty->template_resource, ':'));
    if ($resourcePrefix === $templatePrefix) {
        $basename = basename($smarty->source->name, '.tpl');
    } else {
        $basename = basename($smarty->template_resource, '.tpl');
    }

We probably find a better and more elegant solution...

[PierreRambaud]

I found this one, tell me what do you think?

    $filename = $smarty->template_resource;
    if (!isset($smarty->inheritance->sourceStack[0]) || $filename === $smarty->inheritance->sourceStack[0]->resource) {
        $filename = $smarty->source->name;
    }

    $basename = basename($filename, '.tpl');

[202-ecommerce]

Thank you. We will check it on several platform tomorow and will be back after the testing session.

[PierreRambaud]

I think you solution is maybe for logic and easier to explain than mine ^^
Tell me when you think your solution or mine is working for you and with your themes.
I'll be glad to review and merge it for the 1.7.7 :smile_cat:

[kpodemski]

it's regression so... 1.7.6 right?

[PierreRambaud]

It's not a regression, the problem exists since an old version. The fix on 1.7.6 fix only one problem but not all.

[202-ecommerce]

@PierreRambaud

We test both solutions, and both seems fix the bug without assurance of no regression on specific cases...
My solution treat the symptoms and come from nowhere, yours looks like straightforward.
Mayby a comment helps developers to undersand the origin of this condition and maintain this code.

But the simple fact that I can't explain exactly the reason why this condition is needed (bug smarty, ... ?) is suspicious to be at ease with the solution...

[PierreRambaud]

As I read, it's how Smarty find the current source. Which is in our case, wrong.
Can you create a pull request with your fix:

$resourcePrefix = substr($smarty->source->resource, 0, strpos($smarty->source->resource, ':'));
$templatePrefix = substr($smarty->template_resource, 0, strpos($smarty->template_resource, ':'));
$filename = $resourcePrefix === $templatePrefix ? $smarty->source->name : $smarty->template_resource;
$basename = basename($filename, '.tpl');

And we'll try to get feedback from everyone :)

[202-ecommerce]

Done, I just changed $smarty->source->name by $smarty->source->resource to be more coherent between both conditions.

[202-ecommerce]

You'll found here a new version of the zip with and other case of nesteed includes.
issue-14470.zip

Fix of @PierreRambaud looks fix both controllers.

We will update the pull request.