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).
- 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').
- 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).
- 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.
| Comment | File | Size | Author |
|---|---|---|---|
| #55 | linkit-3041045-55-merge.patch | 11.07 KB | kobe wright |
| #52 | linkit-allow-linkit-url-converter-filter-to-generate-links-3041045-52.patch | 7.34 KB | v.dovhaliuk |
| #35 | 3041045_35.patch | 6.77 KB | nicodh |
Issue fork linkit-3041045
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
berdirI've had the same requirement in a project but I think this should be configurable, whether the link should switch to the language of the referenced entity or if it should stay in the current.
Comment #3
anybodyAlso see #2886455: Multilingual support: Allow linking to specific translations
Comment #4
chris matthews commentedComment #5
bohus ulrychHi all,
I'm also fighting with this.
I have Node1 in English only with link to the Node2, which exists in English and French. If I'm displaying Node1 on the French version of the site, link is always created for the English version of Node2.
I think this is because langcode of such "French" node is still en. This is then applied in the LinkitFilter.php
public function process($text, $langcode) {
If I rewrite $langcode with the current interface language
$langcode = $language = \Drupal::languageManager()->getCurrentLanguage()->getId();
then my link is created correctly to the French version of Node2.
Do I miss something? Or is there any better way how to do it? I was checking #2886455 but I think it tries to solved another problems.
Thanks
Comment #6
pobster commentedThe reason is because although the filter sets the entity to be the translated version;
...when it comes to making the link, it doesn't do anything with that information and just generates "a link".
https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Entity!EntityBase...
It needs the langcode passed into the method to utilise it. I've attached a patch which I made completely separately as I've only just seen this issue ... it's remarkably similar though!
Comment #7
berdir$entity does use the active language with toUrl(), see https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21..., where it passes along the language().
So this is only a problem if there is no translation of $entity for $langcode.
Passing attributes along like this is an API change, but you wouldn't need to do that, you could just set the option on the returned Url object.
Not sure if we should do that without making it configurable as it is a bit of a grey area between bugfix and behavior change IMHO.
Comment #8
bkosborneAgreed this probably something that needs to be configurable given the maturity of the module. Hard to sell a change like this without a way to revert it to how it works now. That said, I agree the current behavior feels wrong.
Currently linkit is outputting links using the translated language of the entity being viewed.
Scenario:
Languages "English" and "Spanish" on site ("English" is default)
node/1 has only an "English" translation.
node/2 has both "English" and "Spanish" translations.
On node/1, there is a link to /node/2 via LinkIt.
When viewing /es/node/1 (Spanish URL), the link to /node/2 will be output as:
Current behavior: /node/1 (the English version), because LinkIt uses the translation of entity being viewed. And since the translation being viewed is English, it will always output target links in English. Basically the Spanish URL context is ignored.
New behavior: /es/node/2 (the Spanish version), because LinkIt will use language context.
Comment #9
bkosborneComment #10
bohus ulrychFYI Patch #6 is not working for me. Still only solution which I'm using is overwrite $language in "
public function process($text, $langcode)" as mentioned in #5Comment #11
bohus ulrychHello, still active topic for me.
There are many combinations of language values : nodes can be translated or not, attached Paragraphs may be translated or not, linked page may be translated or not ...
It seems that now works for me in all cases patch #6, but with additional modification - overwriting $langcode in the process():
Tested with Drupal 9.2.4
Comment #12
ben_a commented@bohus you're exactly right. I wish I'd read your comment before debugging and creating my own patch from #6. Here's my version, which is a copy of #6 with the adding of to the process() you've mentioned.
Comment #13
pere orgaLatest patch has whitespace damage and cannot be applied. It also has an added comment that does not follow code standards. And it does not address the previous comments in #7.
I tried to improve the issue's title and description.
As far as I can understand, a remaining problem is that the linked entity may not have a translation in the current language, or that it may be unpublished. In both cases, I think leaving the link to /node/NID would make more sense.
(It could be argued that in these cases the link should be removed because if the visitor does not have access to it, there is no value in clicking on it. But I guess this could be a separate issue - and likely affects the published status of content in a single language, or when the node does not exist at all...)
Comment #14
nehajyoti commentedI tried apply patch #12 and it says "
"
Comment #15
paul rowell commentedHeya,
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:
Comment #16
paul rowell commentedUpdating status
Comment #17
bogdan.dinu commentedI'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").
Comment #19
bogdan.dinu commentedre-roll of patch #17 with coding standards fixes
Comment #20
bogdan.dinu commentedprevious patch was broken. please use this one.
Comment #21
RoWaBo commentedComment #22
ludo.r#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:
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.
Comment #23
mark_fullmerGood point. The proposal in #2886455: Multilingual support: Allow linking to specific translations 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.
Comment #24
bogdan.dinu commentedI 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.
Comment #25
wim leersI'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:
\Drupal\linkit\Plugin\Filter\LinkitFilter::process($text, $langcode)uses$langcodeto load the appropriate translation of the entity:$entity = $this->entityRepository->getTranslationFromContext($entity, $langcode);$langcodecame from\Drupal\filter\Element\ProcessedText::preRenderText()invoking filter plugins, and it got that itself from$langcode = $element['#langcode'];.\Drupal\text\Plugin\Field\FieldFormatter\TextDefaultFormattergetting 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!
Comment #26
mark_fullmerWould a more encompassing solution than this issue be taking the approach in #2886455: Multilingual support: Allow linking to specific translations?
Put simply: this current issue proposes to provide the *current* language of the entity being edited in the autocomplete suggestions, while #2886455: Multilingual support: Allow linking to specific translations proposes providing autocomplete results from *all* translations.
Comment #27
ben_a commentedI couldn't get the patch from #20 to apply to Linkit version 6.0.2 so I re-rolled it.
Comment #28
bohus ulrychHi 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.
Comment #29
mkalkbrennerWe 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-pathbut to/de/product-pathto stay on the German user-interface.The patch in #27 solves that issue (partly).
"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.
Comment #30
mkalkbrennerI completed the patch so that
Canonicalsubstitution uses the current languge. I also fixed theSubstitutionInterface.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.
Comment #31
mkalkbrennerI accidentally removed a commit. Here's the adjusted patch.
Comment #34
mark_fullmerComment #35
nicodh commentedRe-rolled with config schema mapping added.
Comment #38
james.williamsI'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?
Comment #43
james.williamsThere 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.
Comment #44
james.williamsAs 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?
Comment #45
mark_fullmerThis 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 #2886455: Multilingual support: Allow linking to specific translations 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.
Comment #46
berdirAdding 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.
Comment #47
james.williamsThanks for the encouragement @mark_fullmer and the wise warning @berdir! I suppose the caller (LinkitFilter) setting 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 :)
Comment #48
james.williamsOK, 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
$langcodeis '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.
Comment #49
james.williamsPushed approach 1 for now, as that is the path of least resistance and is along the lines of what Berdir had already suggested before.
Comment #50
gillesv commentedI applied the commits to LinkIt v7.0.2 and it seems to work.
This option is very much necessary when you use the "Language Hierarchy" module with the Linkit filter. That module enables you to place languages in a hierarchy so that for example UK English (en-gb) inherits the translations from English (en). If you use the Linkit filter "Linkit URL converter" without this option/patch, the clearn URL's the filters provide will often be wrong. E.g. if the site interface is currently in UK English (en-gb) and you link to a node that is in the English translation (en), Linkit will link to "/en/..." instead of the desired "/en-gb/...".
There is one issue I noticed: the option "Use the current content language to generate a url (rather than the language of the referenced entity)" is always checked in the text format form. Even if I uncheck it and save it, after a refresh it will be checked again.
Comment #51
james.williams@gillesv good spot! That was introduced back up in comment 17. I've added a fix in the MR.
Comment #52
v.dovhaliuk commentedRerolled patch for the
7.0.4version.Comment #54
kobe wright commentedMerged 7.x into the merge request since there were conflicting changes.
Also adding a patch.See comment #55 for correct patchComment #55
kobe wright commentedAttached the wrong file in comment #54, attaching the correct one now
Comment #58
vasikeit seems it works ... MR rebased with the latest.
for me it looks ok ... but let's try other opinions/reviews.