Allow "Linkit URL converter" filter to generate links based on the language of the current page, rather than the language of the referenced entity

Created on 18 March 2019, over 5 years ago
Updated 23 August 2024, 3 months ago

Problem:
Node links are replaced with aliases from the original language when the translated version no longer exists.

How to reproduce:
Let's say we have a multilingual site in English and Danish languages with nodes being translated, and each language links to different domain (e.g. example.com and example.dk).

  1. Create two nodes with url aliases (e.g. 'Node #1' -> '/node-1' and 'Node #2' -> '/node-2') and translate them to Danish language ('Danish Node #1' -> '/danish-node-1' and 'Danish Node #2' -> '/danish-node-2').
  2. Then in 'Danish Node #1' body place link to 'Danish Node #2'. At this point when viewing content everything is ok, link to 'Danish Node #2' is replaced with aliased version in Danish language (http://example.dk/danish-node-2).
  3. Now delete 'Danish Node #2' translation leaving just the English version. Then when viewing 'Danish Node #1' we get node alias to English version (http://example.com/node-2).

Expected: Onward links within English content used as a fallback for missing Danish content, should be to Danish content, in order to preserve the requested language.
Currently: Onward links within English content used as a fallback for missing Danish content, should be to English content, which preserves what the displayed English content originally linked to.

(But there may be cases where the current behavior is reasonable to expect.)

Proposed resolution

Generating the link in the language of the page hosting the link (Danish), rather than that of the linked entity (now only English) would resolve this, as the generated 'plain' link (http://example.dk/node/2 for the example scenario) would allow any useful redirects that replaced the content in that language to be followed. Or if there is no redirect, users following the plain link would at least be kept within the same language (even if Drupal shows the English translation content as a fallback, crucially it would be within a Danish interface).

There is a case for the existing behaviour too though, so this change could be implemented as a new configuration option allowing site owners to opt-in to whichever they prefer.

✨ Feature request
Status

Needs review

Version

7.0

Component

Code

Created by

πŸ‡±πŸ‡ΉLithuania k-l

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¬πŸ‡§United Kingdom Paul Rowell

    Heya,

    We needed this func/feature for a language fallback we have in place. I've taken the latest patch and added a configurable field in the filter:

  • Status changed to Needs review almost 2 years ago
  • πŸ‡¬πŸ‡§United Kingdom Paul Rowell

    Updating status

  • πŸ‡·πŸ‡΄Romania bogdan.dinu

    I've updated the last patch with a simple check on the default value for the form element because it was throwing a warning (Undefined array key "use_current_language").

  • The last submitted patch, 17: linkit-3041045-localise-link-config-17.patch, failed testing. View results β†’
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Status changed to Needs work almost 2 years ago
  • πŸ‡·πŸ‡΄Romania bogdan.dinu

    re-roll of patch #17 with coding standards fixes

  • πŸ‡·πŸ‡΄Romania bogdan.dinu

    previous patch was broken. please use this one.

  • Status changed to Needs review almost 2 years ago
  • πŸ‡§πŸ‡ͺBelgium ludo.r Brussels

    #20 works on my side and my use case, but I guess it's worth to mention that it does not exactly what the option claims:

    Use the current entity language to generate a url (rather than the language of the referenced entity)

    In fact it just gets the current language page rather than the referencing entity language.
    This might be a problem if you want to show items in other languages than the current page language.

  • πŸ‡ΊπŸ‡ΈUnited States mark_fullmer Tucson

    This might be a problem if you want to show items in other languages than the current page language.

    Good point. The proposal in ✨ Multilingual support: Allow linking to specific translations Needs review provides UI-based options for determining what behavior is desired. I think it would be good to either look at these approaches together and determine whether one can resolve the goal of both issues. If not, probably this issue needs some kind of configuration option to be able to show items in other languages.

  • πŸ‡·πŸ‡΄Romania bogdan.dinu

    I also ran into this issue recently and I think the best option would be to enable the language option on the link widget.
    The link field supports for the language option and it will automatically generate the link in the selected language. The language selector can default to the current page language but the user will have the possibility to change it.
    Unfortunately I didn't have time to implement it yet.

  • Status changed to Postponed: needs info over 1 year ago
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    I'm very confused by this issue, and surprised it even exists! It may be because I'm not at all familiar with the "substitution plugin" part of the codebase.

    Here's why I'm confused:

    1. βœ… \Drupal\linkit\Plugin\Filter\LinkitFilter::process($text, $langcode) uses $langcode to load the appropriate translation of the entity: $entity = $this->entityRepository->getTranslationFromContext($entity, $langcode);
    2. βœ… That $langcode came from \Drupal\filter\Element\ProcessedText::preRenderText() invoking filter plugins, and it got that itself from $langcode = $element['#langcode'];.
    3. βœ… That in turn comes from \Drupal\text\Plugin\Field\FieldFormatter\TextDefaultFormatter getting it from the containing field item on the referencing entity itself: '#langcode' => $item->getLangcode(),

    So … are we sure that this is a bug in/requires changes in the LinkIt filter plugin?

    Because that plugin already has explicit translation-related test coverage specifically testing that the translation of the linked entity matches the language of the linking entity: \Drupal\Tests\linkit\Kernel\LinkitFilterEntityTest::testFilterEntityTranslations().

    A failing test would make all the difference here!

  • πŸ‡ΊπŸ‡ΈUnited States mark_fullmer Tucson

    I also ran into this issue recently and I think the best option would be to enable the language option on the link widget.
    The link field supports for the language option and it will automatically generate the link in the selected language.

    Would a more encompassing solution than this issue be taking the approach in ✨ Multilingual support: Allow linking to specific translations Needs review ?

    Put simply: this current issue proposes to provide the *current* language of the entity being edited in the autocomplete suggestions, while ✨ Multilingual support: Allow linking to specific translations Needs review proposes providing autocomplete results from *all* translations.

  • I couldn't get the patch from #20 to apply to Linkit version 6.0.2 so I re-rolled it.

  • πŸ‡¨πŸ‡ΏCzech Republic Bohus Ulrych Pilsen (Czechia)

    Hi all,
    FYI patch #27 can be applied on the version 6.1.2 too.

    I'm on the English node, creating link to the English version of another node. Both pages are untranslated.
    In the Japanese version of the site the link is correctly using path with language prefix (/jp/node/nid)
    (don't forget to check "Use the current entity language ... " on the CKEditor / Linkit URL convertor settings)

    Note:
    In my case, I'm not happy to see in URL /jp/node/nid for untranslated pages. Therefore my own customization creates aliases for untranslated pages based on the original language. So it could be like /jp/my-title. But in such case, after applying patch, I will get URL /my-title (to the original language) instead of expected version with prefix /jp/my-title
    I found that this is because in this version of the patch were removed modification in Substitution plugins (e.g. Plugin/Linkit/Substitution/Canonical.php) - compared to the #6. After applying this part of patch it works according to my expectations.

  • Status changed to Needs work 10 months ago
  • πŸ‡©πŸ‡ͺGermany mkalkbrenner πŸ‡©πŸ‡ͺ

    We faced a related issue. We have untranslatable commerce product entities with language undefined path aliases (which work for all languages).
    All the links are generated using the site's default language 'en' instead of the the current content language.
    For example, if you're on the German translation of a node you don't want to be sent to /en/product-path but to /de/product-path to stay on the German user-interface.

    The patch in #27 solves that issue (partly).

    +++ b/src/Plugin/Filter/LinkitFilter.php
    @@ -107,12 +119,18 @@ public function process($text, $langcode) {
    +              $langcode = $this->languageManager->getCurrentLanguage()->getId();
    

    "current language" is not well defined. Is is the interface language or the content language.

    If you use one of the contrib modules that allow editing multiple translations in parallel, it would be better to explictly get the language from the current form object.

  • Status changed to Needs review 10 months ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.2.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update 10 months ago
    82 pass, 2 fail
  • πŸ‡©πŸ‡ͺGermany mkalkbrenner πŸ‡©πŸ‡ͺ

    I completed the patch so that Canonical substitution uses the current languge. I also fixed the SubstitutionInterface.

    I didn't address the edge case of multiple translation in one form, because it might be out-of-scope for this module.

    With this patch applied, LinkIt works for us.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.2.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update 10 months ago
    82 pass, 2 fail
  • πŸ‡©πŸ‡ͺGermany mkalkbrenner πŸ‡©πŸ‡ͺ

    I accidentally removed a commit. Here's the adjusted patch.

  • The last submitted patch, 30: 3041045_30.patch, failed testing. View results β†’
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • The last submitted patch, 31: 3041045_31.patch, failed testing. View results β†’
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • πŸ‡ΊπŸ‡ΈUnited States mark_fullmer Tucson
  • πŸ‡«πŸ‡·France nicodh

    Re-rolled with config schema mapping added.

  • πŸ‡¬πŸ‡§United Kingdom james.williams

    james.williams β†’ made their first commit to this issue’s fork.

  • Pipeline finished with Failed
    3 months ago
    Total: 138s
    #261668
  • πŸ‡¬πŸ‡§United Kingdom james.williams

    I've opened MR !62 starting from the latest patch in comment 35, and then added a commit to improve it based on the comments 22 and 29 - specifying to use the content type of language (as opposed to interface language and the confusing 'entity language' wording).

    We should probably be using issue forks instead of patches going forwards anyway? I see the phpunit test job in the 6.1.x branch is failing to run successfully. Should we just move to targeting 7.x where the tests appear to work?

  • Pipeline finished with Success
    3 months ago
    Total: 294s
    #261701
  • πŸ‡¬πŸ‡§United Kingdom james.williams

    james.williams β†’ changed the visibility of the branch 3041045-7x-allow-using-current-language to hidden.

  • πŸ‡¬πŸ‡§United Kingdom james.williams

    james.williams β†’ changed the visibility of the branch 3041045-7x-allow-using-current-language to active.

  • πŸ‡¬πŸ‡§United Kingdom james.williams

    james.williams β†’ changed the visibility of the branch 3041045-allow-linkit-url to hidden.

  • πŸ‡¬πŸ‡§United Kingdom james.williams

    There we go; I've re-rolled the changes to go against 7.x in MR !63 and the test pass there. So I've hidden the MR for the 6.1.x branch (and all but the most recent patch file). But we haven't actually adjusted tests to start covering this issue yet.

  • Pipeline finished with Failed
    3 months ago
    Total: 268s
    #261741
  • Pipeline finished with Failed
    3 months ago
    Total: 258s
    #261746
  • Pipeline finished with Success
    3 months ago
    Total: 257s
    #261753
  • Status changed to Needs review 3 months ago
  • πŸ‡¬πŸ‡§United Kingdom james.williams

    As per comment 25 from Wim Leers, a failing test would make all the difference here .... well, we now have a test that does indeed fail in the tests-only feature, and passes with all the changes :) See https://git.drupalcode.org/issue/linkit-3041045/-/pipelines/261753 !

    I've also updated the issue summary, hopefully that's now sufficiently clear and correct?

  • πŸ‡ΊπŸ‡ΈUnited States mark_fullmer Tucson

    well, we now have a test that does indeed fail in the tests-only feature, and passes with all the changes :)

    This is wonderful! Thanks so much. As a maintainer of the Linkit module, I am open to adding this to the module, given that it is an opt-in configurable feature. I just want to make sure everyone who is invested in ✨ Multilingual support: Allow linking to specific translations Needs review as another multilingual feature request is still accommodated. I'll leave a comment on that issue for the followers to review this issue's updated description and where it's landed to allow a chance for more feedback before proceeding.

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    Adding a new parameter to SubstitutionInterface::getUrl() breaks BC.

    7.x is technically only in alpha, but it's still challenging. There's an open issue in media_entity_download where people then get a fatal error when using this patch.

    Doing this through the filter means this possibility is not available to link fields or it would need to be implemented separately there.

    We needed this too in a project, and implemented it with a custom substitution plugin instead.

  • Status changed to Needs work 3 months ago
  • πŸ‡¬πŸ‡§United Kingdom james.williams

    Thanks for the encouragement @mark_fullmer and the wise warning @berdir! I suppose the caller (LinkitFilter) could set the options on the returned URL instead of inside SubstitutionInterface::getUrl() might be fine, I'll test. I see that was actually suggested back in comment 7!

    I'll also take a look at whether this only affects the filters, or if it's actually needed for link fields too. I suppose the canonical substitution plugin could even be swapped out for an equivalent plugin class that always uses the current language's translation. Anyway, these are thoughts I'll take away.

    P.S. If no-one hears back from me on this for days, anyone else is welcome to pick this up again too, you can assume I got distracted by summer sun :)

  • πŸ‡¬πŸ‡§United Kingdom james.williams

    OK, I see that yes, the field formatter is affected as well as the filter plugin.

    @mark_fullmer @berdir which of the following approaches would you prefer:
    1) Both the Filter & Formatter plugins just set the language option on the URL returned from the substitution plugin (duplicate code, but preserves BC and perhaps simplicity?)
    2) Make the existing Canonical substitution plugin configurable, to optionally always use the current page's content language for the language option (rather than passing that in from the caller). Though that would then mean having to pass around plugin configuration.
    3) Provide an additional substitution plugin which always uses the current page's content language for its returned URLs.

    If you don't want that possible BC break in the current state of the MR, then I guess I'd lean to option 3.

    Since my investigations helped me understand further, I'll answer Wim's comment 25 as to why this is an issue. Sorry if this is just unnecessary detail!
    This is an issue because when a translation is missing, Drupal tends to fall back to showing another translation (e.g. the initial language version of the entity). So the $langcode is 'correct' in that it refers to the language of the content being displayed - but that's not necessarily the requested language for a page's content. (In the issue summary's example scenario, Danish is requested, but when that translation is missing, English is picked and then used throughout.) So in a sense this feature request is about respecting the requested language for onward links beyond the page's own main content - or at least having an option to choose between that or deferring entirely to Drupal's chosen fallback behavior. In summary:

    A. Onward links within English content used as a fallback for missing Danish content, should be to Danish content, in order to preserve the requested language.
    vs
    B. Onward links within English content used as a fallback for missing Danish content, should be to English content, in order to preserve what the displayed content originally linked to.

    I've added that comparison to the issue summary since that's hopefully a useful articulation of the issue.

  • Pipeline finished with Success
    3 months ago
    Total: 301s
    #262630
  • Pipeline finished with Success
    3 months ago
    Total: 293s
    #262641
  • Status changed to Needs review 3 months ago
  • πŸ‡¬πŸ‡§United Kingdom james.williams

    Pushed approach 1 for now, as that is the path of least resistance and is along the lines of what Berdir had already suggested before.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 138s
    #312323
Production build 0.71.5 2024