Steps to reproduce.

Enable entity_translation entity_translation_i18n_menu
Enable a second language.
Set node type and menu as translatables.

Translate a node to both languages setting including a translation of the menu link description that includes a character like '&'.

Save the translations and edit the translation that is not the source.

You will notice that the ampersand is now "&".

Everytime you save the node the & will be expanded to & creating strings like "&" ad infinitum.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rodrigoaguilera created an issue. See original summary.

rodrigoaguilera’s picture

Status: Active » Needs review
FileSize
2.8 KB

Since we are displaying a string for a form it doesn't need to be sanitized but that happens in the i18n_menu.

My patch copies the the two functions from i18n_menu involved
_i18n_menu_link_localize
_i18n_menu_link_description

and disables sanitization for the description.

rvilar’s picture

It works for me!

stefanos.petrakis@gmail.com’s picture

Thank you both for (a) identifying and working on this issue and (b) providing feedback on the proposed solution.

Unfortunately, the solution provided by the patch in #2 has some weak points, most importantly the copy-n-paste approach.

Additionally, after looking deeper into the whole issue, I believe the problem lies on the i18n side, and Entity Traslation should not try to amend that problem, but we should rather try to solve this inside i18n.
There is a related issue and a patch available over at #2973611-2: _i18n_menu_link_description(): String sanitization should be disabled for the menu item's description to be displayed correctly that should solve this problem. Actually there are two different problems with the same origin, one has to do with the rendering of the description of translated menu links, the second one is the one reported here.
Applying that patch against i18n solves both problems.

I updated the entity_translation_i18n_menu tests in order to cover the reported problem as well as the rendering case and I am submitting the changes here.
N.B.: The updates tests should fail if the patch from #2973611-2: _i18n_menu_link_description(): String sanitization should be disabled for the menu item's description to be displayed correctly is not applied.

Next steps:

Till the related i18n issue and related patch are committed, or an alternative solution appears, we should keep this issue open.

joseph.olstad’s picture

Status: Active » Needs review

trigger runners

Status: Needs review » Needs work

The last submitted patch, 4: 2674696-tests-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

joseph.olstad’s picture

Status: Needs work » Active
joseph.olstad’s picture

ok, yes, I will commit the related i18n patch and re trigger these tests.

joseph.olstad’s picture

Status: Active » Needs review

retriggered all tests after i18n commit

Status: Needs review » Needs work

The last submitted patch, 4: 2674696-tests-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

joseph.olstad’s picture

I just checked the job console

the job is pulling the latest tagged release of i18n, not the latest dev of i18n
so, will have to tag a release of i18n to get this test to pass.

Stefanos, have you ran these tests locally?
that would work, if the dev version of i18n was pulled locally, and run this patch against et

stefanos.petrakis@gmail.com’s picture

Hi @joseph, I did run them locally yes! With the next tagged release of i18n, the tests should pass.
And according to the bot, the patch with the new tests need some coding standard corrections.
"Needs work" seems good for now.

joseph.olstad’s picture

Status: Needs work » Reviewed & tested by the community

I've tagged and released i18n 7.x-1.25

I'd say this patch above is ready for commit.
Thanks

stefanos.petrakis@gmail.com’s picture

Status: Reviewed & tested by the community » Needs review

Thanks @joseph, let's check with the bot first.

joseph.olstad’s picture

looks good so far, I just queued a few extra tests

stefanos.petrakis@gmail.com’s picture

Status: Needs review » Closed (works as designed)

New tests are green, closing this "works as designed". Many thanks for the help on this to everyone involved.

joseph.olstad’s picture