magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.37k stars 9.28k forks source link

js-translation.json does not pick up strings in *.phtml files #33245

Open amenk opened 3 years ago

amenk commented 3 years ago

Follow up of https://github.com/magento/magento2/issues/8508#issuecomment-861357969

Background

Magento automatically picks up strings which should be supplied for the frontend translation via component via js-translation.json. This misses some strings, leaving hard-to-debug problems with translations to other languages.

Summary (*)

The process of fetching strings for js-translation.json has problems in 2.3.5 - and I am pretty sure it's still a problem in the dev master.

We struggled to translate the string "items" for the knockout component "minicart".

Steps to reproduce (*)

  1. Create a new custom module.
  2. Set Locale language to French (France) in Admin configurations Stores->Configurations->General->Locale Options.
  3. Add French translation for ‘foo bar 123’ in Admin Panel to “barre de foo 123”
  4. Keep string in newly created module phtml file to check if string is getting translated.
  5. Flush cache and check module on frontend.

Result: String is not getting translated with knockout js tag. It is displayed as ‘foo bar 123’ only.

Examples (*)

The extraction of the files happens here:

https://github.com/magento/magento2/blob/32fda1cba6ebd1bd7837d01dc34accf67469d560/app/code/Magento/Translation/Model/Js/DataProvider.php#L97-L102

The translatable string "items" is introduced here:

https://github.com/magento/magento2/blob/32fda1cba6ebd1bd7837d01dc34accf67469d560/app/code/Magento/Checkout/view/frontend/templates/cart/minicart.phtml#L22

But the above extraction method does not count *.phtml files in - so translations like this are lost. (it eventually might still end up in the js-translation.json if the same string is used somewhere else in the theme, so such would cover up this bug)

Proposed solution

Include \Magento\Framework\App\Utility\Files::getPhtmlFiles or something similar in the list of files - but this might have a performance impact.

Or remove the whole automatic string extraction and add a manual list of strings which should be included in js-translation.json - this would make the whole process much simpler and less magic.

-- Please provide Severity assessment for the Issue as Reporter. This information will help during Confirmation and Issue triage processes.

m2-assistant[bot] commented 3 years ago

Hi @amenk. Thank you for your report. To help us process this issue please make sure that you provided the following information:

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, please, review the Magento Contributor Assistant documentation.

Please, add a comment to assign the issue: @magento I am working on this


:clock10: You can find the schedule on the Magento Community Calendar page.

:telephone_receiver: The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

:movie_camera: You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

:pencil2: Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

amenk commented 3 years ago

As code changes are necessary / it's a developer experience issue, I do not know how to reproduce that by the way. But from looking at the code, the issues is pretty clear.

mrtuvn commented 2 years ago

i think actually issue knockout not translated text in phtml file. Php translate still work as we expected but not in the case that used syntax ko i18n. I just found that text you mentioned only happen in one place and i don't see core team do that again in other place.

Another thing i found start from magento 2.4 i dont see file js-translation.json contain anything text when compare with previous version (2.3.x). File js-translation have contain some texts tested in local with preconditions translate_strategy => dictionary

m2-assistant[bot] commented 2 years ago

Hi @engcom-Lima. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Lima commented 2 years ago

Hi @amenk,

Since this issue is originally 3 years old and were probably tested on old versions of Magento. So can you please check whether this issue still exists on 2.4-develop instance ?

If yes, please provide steps to reproduce on 2.4-develop instance.

For checking the issue thoroughly, it is very important that the issue should be reproducible. Then only we’ll be able to find a solution to the issue.

amenk commented 2 years ago

@engcom-Lima No, this issue is not 3 years old. We tested on 2.3.5 and it seems a general problem. Can you provide the commits where this was fixed?

I am pretty sure the issue is still there, as in https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Translation/Model/Js/DataProvider.php#L97 also .phtml files are left out from the string collection and in https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Checkout/view/frontend/templates/cart/minicart.phtml still strings are introduced in .phtml

I currently don't have the time to reproduce this on 2.4, but I hope it's not getting closed, because if it is not fixed, it will frustrate more developers in the future.

Short steps to reproduce would be:

This is not easy to reproduce on the Magento 2.4 Dev instances created by the GitHub bot, as we don't have SSH access there.

mrtuvn commented 2 years ago

https://devdocs.magento.com/guides/v2.4/frontend-dev-guide/translations/translate_theory.html Seem knockout not support translate from phtml if you see this document. I don't see any mention in this doc translate by knockout syntax only work for template ui component or underscore template (html files). So if we mix ko template in phtml magento may not detect it Maybe this start point for start investigate https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/i18n.js

amenk commented 2 years ago

Interesting catch, yeah, so it is having a flaw by design. I believe the problem is not in https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Ui/view/base/web/js/lib/knockout/bindings/i18n.js but in the collection of the strings in the backend-code.

engcom-Lima commented 2 years ago

Hi @amenk,

I tried to reproduce this issue but did not find any problems in translating ‘foo bar 123’ into any language that I wish.

Here is what I did:

  1. I created a new module to test it out.
  2. I kept a string "foo bar 123" in the .phtml file. As per documentation kept it like __('Testing - foo bar 123') so that it can be translated properly. I have not added translation for 'Testing - ' so it is displayed in English only.
  3. I sat local language to French (France) in Admin configurations.
  4. I added French translation for it in Admin Panel like “barre de foo 123”
  5. Flushed cache and checked on frontend. Result came with expected translations.

Please find the attached screenshot of the same. String is getting translated as expected:

Screenshot 2021-09-13 at 1 26 18 PM

Can you please try at your end and let me know if you are still facing this issue or update what you did differently at your end to get this issue ?

mrtuvn commented 2 years ago

@engcom-Lima you have to translate in file phtml use knockout syntax instead for reproduce. That is correct case author try to mention here https://github.com/magento/magento2/blob/32fda1cba6ebd1bd7837d01dc34accf67469d560/app/code/Magento/Checkout/view/frontend/templates/cart/minicart.phtml#L22 <= this file is example. See the code highlighted with ko i18n

amenk commented 2 years ago

Yes, exactly how @mrtuvn clarified: Normal .phtml translation on PHP level with the __() function of course works, knockout translation in .html also works. The problem is about the knockout translation in .phtml files.

engcom-Lima commented 2 years ago

Hi @amenk, @mrtuvn,

Thank you for the clarification.

Verified the issue and issue is reproducible on 2.4-develop branch.

I added string <!-- ko i18n: 'foo bar 123' --><!-- /ko --> in the same module's .phtml file which I created earlier but the translation did not worked with ko syntax.

Language Translation is working with php tag and with knockout translation with .html also but not working with ko syntax. Based on it, confirming this issue.

Thanks

github-jira-sync-bot commented 2 years ago

:white_check_mark: Jira issue https://jira.corp.magento.com/browse/AC-1211 is successfully created for this GitHub issue.

m2-assistant[bot] commented 2 years ago

:white_check_mark: Confirmed by @engcom-Lima. Thank you for verifying the issue.
Issue Available: @engcom-Lima, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

mikimpe commented 1 year ago

It appears that this issue affects also the translation of strings in the js section of a phtml file using the 'mage/translate' feature. In this case, as a workaround, you can translate the string using PHP and then pass it to the js, for example like this:

let translatedString = "<?php echo __('Translate this string'); ?>";