Rendered HTML Output doesnt respect activeLanguage completely

Can you specify in a bit more detail what parts of the HTML output use the wrong language?
Also, if you say yourself this might be a general issue, did you also raise it in the Core issue queue? Would of course be great if this could be fixed there.
Otherwise: What do you suggest? Switching out the language negotiator temporarily in our plugin? Might work, but sounds risky. Don’t have any other ideas, though, so it seems our best shot. Can you provide a patch?

I've come across the same issue with D7. There we've the same problem - passing in the language to entity_view() won't necessarily have effect on lots of things e.g. the rendering of links (url() still relying on $GLOBALS['language_url'])

Switching out the language negotiator temporarily in our plugin? Might work, but sounds risky.

For D7 this seems the only way too. Even thought we can't guarantee that we won't have weird effects due static caches which are generally not language aware and hence could make trouble. However, the output is definitely better with the adjustment.

Unfortunately can only provide a D7 patch for now.

Timely addition. Thanks @vierlex!

I'm using it in a dev environment and it seems to be working for me on 8.7.1 with search api 8.13.

I'll let you know if I run into any issues.

Thanks!

Taking a look at those failing tests.

I've found a way to swap out the language negotiator without having to add language as a dependency. This resolves most of the test failures, but there are still some failures remaining when I run the tests locally. I'm out of time today, so posting my work in progress for now.

I think this should fix the last failing tests.

As always, I haven't figured out yet why they won't apply when running through the Testbot :(

Glad you could make it work! You’ve probably realized it by now, but the key is to have something that will be accepted by git apply. Using git diff is the simplest way to make sure of that.
Your initial patch missed the usual a/*/b/* prefixes for file paths, so git apply removed the leading src/ part of the paths – which of course breaks them. Not sure what was wrong with your later patches – maybe not using the current dev version?

In any case, thanks a lot for posting your work!
Some of the code was not quite perfect in my opinion, and it also ignored the Drupal coding standards partially. Please see/test/review my attached revision!
Two more things:

1. This still needs a regression test for verifying that a) there was a problem and b) the patch fixes it. This will make sure we’re actually solving a problem, and that we don’t break this again in the future. Probably, it can just live in the existing RenderedItemTest class – install the language module, set the current language to English and then try to render a French translation of a node with comments (or term references, or something like that).
2. As I’m still not convinced this won’t break things for some sites, and as it might not actually be needed on them, should we maybe make this configurable on the processor? Or would you say, as long as the Language module is present it’s pretty sure people will want this?

@ das-peter/#7: Thanks for posting, looks very good! While tests are not necessary for D7, my other comment still applies there, too: Do you think we should maybe make this configurable?

“Needs work” status for tests.

@ das-peter/#7: Thanks for posting, looks very good! While tests are not necessary for D7, my other comment still applies there, too: Do you think we should maybe make this configurable?

...should we maybe make this configurable on the processor? Or would you say, as long as the Language module is present it’s pretty sure people will want this?

Thanks for the feedback :)
I think it's fair to be cautious regarding existing sites. Making it configurable sounds like a good way to go. I've update the D7 patch to make the behavior configurable and it also sets a "sensible" default. This means for existing configurations the language switch will be disabled unless explicitly enabled but for new configurations it will be enabled by default upon creation.

Thanks for the updated patch, Peter! Looks very good now.
Just expanded the form field description to hopefully cause less confusion to everyone who didn’t follow this issue and moved the code around a bit to look cleaner to me.
Otherwise, RTBC, if you agree? Thanks again, in any case!

Back to NW for D8 tests.

Otherwise, RTBC, if you agree? Thanks again, in any case!

Sure! Thanks for the review and the pro-active feedback, much appreciated!!

Great, thanks for the feedback!
Committed the D7 patch, then. Thanks again!

D8 still needs such a final push. Any takers?

Thanks for your work, the #20 patch solves my problem of the default language is used while rendering twig files, resulting in wrong tranlslations when the t-filter is used, urls linking to wrong language or untranslated dates. My use case is storing the rendered search results in solr and fetch them from there. (D8.6, Search-API 1.13, Search API Solr Search 3.1)

Edit: I have to correct myself, the t-Filter is still using the default language. A workaround is to pass the String via preprocess or use the trans-function, and pass the now correct available langcode from preprocessor:

{% set link_title %}
  {% trans with {langcode: current_langcode} %}
    Details anzeigen
  {% endtrans %}
{% endset %}

Patch 20 for D8 works well for us when indexing via the UI, however on the command line it fails to make sure that the rendered html output is in the right language.
I was able to track the cause down.
Somewhere after the static negotiator (introduced by the patch) is put into the language manager, the language_negotiator service gets initialised by the service container (it seems to be somewhere in the dependency tree of content moderation's EntityOperations class). And the language_negotiator service initialises the language manager, setting itself again as the negotiator and thus replacing our static negotiator again.

core/modules/language/language.services.yml:

  language_negotiator:
    class: Drupal\language\LanguageNegotiator
    arguments: ['@language_manager', '@plugin.manager.language_negotiation_method', '@config.factory', '@settings', '@request_stack']
    calls:
      - [initLanguageManager]

core/modules/language/src/LanguageNegotiator.php,

/**
   * Initializes the injected language manager with the negotiator.
   *
   * This should be called right after instantiating the negotiator to make it
   * available to the language manager without introducing a circular
   * dependency.
   */
  public function initLanguageManager() {
    $this->languageManager->setNegotiator($this);
  }

To solve this, I've put one line of code before setting up the static negotiator, to initialize the language_negotiatior service. Updated patch attached.

After installing the patch, I'm experiencing the issue described here for some of my nodes: https://www.drupal.org/project/drupal/issues/2869347

We've been using the patch in #29 but has noticed a bug where translations are not rendered in the correct language. After a lengthy debug session I found that the string_translation service is initialized with the default language so we need to update that as well.

I created an issue fork, rerolled the patch from #29 and then implement my changes.

https://git.drupalcode.org/issue/search_api-3035977/-/compare/8.x-1.x......

URL to patch:
https://git.drupalcode.org/project/search_api/-/merge_requests/2.patch

I don’t think there is a need for patches when we already have a fork? Anyways, also no harm in creating an MR before this is completely done. That way, there is also constant automated testing. (Even though something seems to have gone wrong in the last test …)

Anyways, thanks a lot for working on this!
I looked into it, and your change does make sense, even though the comment explains it a bit poorly, and you forgot to reset the language afterwards. I hopefully fixed both of those things now, plus made a few cosmetic changes.

Also, apart from all that, as RenderedItem is a plugin, it uses dependency injection, so we shouldn’t get those services via \Drupal::… but through DI.
So, in addition to the automated tests, this would also need to be fixed before this can be merged.

Created #3190024: Problem with test dependencies when testing issue forks for the test failures.

Good catch!

Here's the original patch from #35. It looks like the MR is updated dynamically, and the MR-patch can no longer be applied cleanly with composer-patches.

Last patch fixes the issue in the OP completely for me using rendered item indexing!

Same, patch from #39 solves the issue — Thanks!
We had the problem, that the UI translations were rendered in the language negotiated by Drupal --> so always German.

Reroll the MR and make a patch for it.

I'm just doing some light work related to #3190024: Problem with test dependencies when testing issue forks.

@fjgarlin: Thanks a lot for your work on that!
However, please do such work in #3396064: (Try to) fix the GitLab CI in the future, it’s quite unrelated to this ticket.

@drunken-monkey 100% you’re right, apologies. I was meant to undo the changes but totally forgot. My bad.

I arrived at this issue following the related ones and just wanted to prove something (which I did) in the related issues.

I’ll undo my commits Monday first thing (away from computer right now).

My changes are now reverted.

#3380219-2: Rendering is not changing the current content and string translation language is a duplicate that contains a patch with a different approach, not needing a new service class (which I’d count as a big plus). Please try it out and see whether this doesn’t also resolve this problem for you.

In either case, though, we still need a regression test for this.

My use case for this fix is a field formatter that uses t(). I created a new branch with the patch from #3380219: Rendering is not changing the current content and string translation language that doesn't create a new service. I haven't been able to figure out how to write a test for it though.

After some additional testing, I've found that the approach in this issue (creating a new language negotiator) works better than the approach from #3380219: Rendering is not changing the current content and string translation language .

The patch from the other issue fixes config and string translations, but entity references are still rendered in the request language. (Specifically - in layout builder, non-translatable custom blocks that reference translatable entities.) The branch from this issue is more complete and fixes all three - config, string, and entity translations.

Uploading a static patch of the MR:

Re-based MR and uploading a static patch.

Sorry about the noise, I just wanted to remove two modules from composer.json, which I think should not be there, and failed ...

Just a tip that I managed to get it to work in Search API Solr, and how to, using the patch in #3380219: Rendering is not changing the current content and string translation language (No, I was wrong.)

Thanks everyone, this MR works, and I get translated labels in a "Rendered HTML output" (rendered_item) field correctly indexed in Search API Solr.

I struggled to get everything set up correctly, Search API, Search API Solr, Drupal's language settings, Views, etc. but finally succeeded, so I'll share some important steps, in case anyone else is also having a hard time, and it's useful.

If you are using "Path prefix" under "URL language detection configuration". Search API will use the context of the page as a filter, so that when pages under example.org/da/ is accessed, Solr will only return items with "sm_language_with_fallback":["da"],. Likewise, if pages under example.org/en/ are accessed, Solr will only return items with "sm_language_with_fallback":["en"],:

These are some of the required steps, to get rendered, translated data indexed correctly in Search API Solr, and extracted, using "Language (with fallback)" as a Views filter:

• Use dev-version of Search API with latest patch in this issue (I assumed wrongly for a while that version 1.38 would work)
• Add the "Language (with fallback)" field in your Solr index
• Important: Enable "Content language detection" under /admin/config/regional/language/detection:

  x Customize Content language detection to differ from Interface text language detection settings

  If "Content language detection" is not enabled, "Content language selected for page" is not an option, in the filter in the View

• Add "Language (with fallback)" as a Views filter, with "Content language selected for page" selected

About the filter options: After trying many things, I tried a systematic approach, using the wonderful Search API Solr Devel to debug the requests and responses. Looking at a node translated into two languages (da and en), which had a Views block attached (using Contextual filters > Content datasource: ID (Default: Content ID from URL)), I noticed that both languages were being requested, which puzzled me -- all nodes showed the block twice, in both languages. I thought I had set up the "Language (with fallback)" filter in my view correctly, by selecting "Danish" and "English", but apparently not ...

Because this request was sent to Solr, under a /da/node/123 page:

(+ss_type:"municipality" +sm_language_with_fallback:("da" "en"))'

I tried some other options in the Views "Language (with fallback)" filter, and when I finally selected "Content language selected for page" it worked, and only either da or en were requested, as desired. Now this was sent to Solr, under example.org/da/ pages:

'(+ss_type:"municipality" +sm_language_with_fallback:"da")'

The "Language (with fallback)" Views filter options:

Configure filter criterion: Search: Language (with fallback)

Operator
x Is one of
  Is not one of

Language
  Select all
  Site's default language (Danish)
  Interface text language selected for page
x Content language selected for page
  Danish
  English
  Not specified
  Not applicable

So I wonder if calling that option something else like "Content language of page" would work better, or something else? The current wording make it sounds like the user is doing something actively ("selected for") which the user isn't ...

GitLab is not doing so well in this issue, it looks like it's stuck, and can't create a new branch with the latest code ...

Anyway, I was wrong about my previous comment, the patch in this issue works. I mixed up the two patches (I thought that 2.patch was in fact 237...)

I am attaching an updated patch.

I think this patch may be a bit closer to match dev.

Just an observation: The default language in my installation is English (under /en), with Danish as the other language, without language prefix (under /) like this:

1. Add Danish language in an existing English installation
2. Open "URL language detection configuration" (/admin/config/regional/language/detection/url)
3. Update "English (en) path prefix (Default language)" to en
4. Remove da under "Danish" and click Save
5. Get message about "The prefix may only be left blank for the selected detection fallback language."
6. Open link in new window and set "Language" to Danish
7. Go back to "URL language detection configuration" which can now be saved

The default language is now Danish at example.com (English is under example.com/en) and I get the text "Velkommen!" on the front page.

From Use URL language detection configuration?

It mostly works, but sometimes, the system somehow gets stuck in its old ways, and only English labels get indexed ... I tried many things like clearing cache, rebuilding Solr index, etc., but this is what seems to work:

Open the edit form for a node under Danish (/node/259/edit), and see English translated field names. If I then clear caches, Drupal "wakes up" and realizes there is a default language (/en), but the fallback language in the current page is Danish. The fields are now rendered in Danish, and now, when I index, the correct translated field labels are used in the rendered HTML fields, in the Solr index content.

Rebased patch for the new dev-release.

This patch still works well, and I have updated the Issue Summary, adding "Rebase" and "Writing regression test" under "Remaining tasks" :)

Once I identified what was the problem, I found that issue and patch #65 fixed the issue! Thank you.

I've created a new branch in the fork, based off origin/8.x-1.x.
In this branch I've applied the patch from #65 and written a regression test (with the help of Claude).
I've verified the regression test fails without the patch, and passes with the patch.

I note the phpunit fails in the latest pipeline run are unrelated to the changes here and appear to relate to #3614340: Update to Twig 3.28.

I've hidden the older branches, merge requests and files for clarity.

Setting to needs review.

Thanks for creating the MR and adding a regression test.
It still feels like a lot of trouble, adding a new service and a service provider class just for the sake of this small problem, but if this works and nothing else will, then I guess we should still do it.

However, I don’t actually quite see why the negoiator has to be a service? Couldn’t we just create the class directly in RenderedItem (maybe in create() so we have proper dependency injection)?
It seems having the service is both extra code and also potentially confusing to others who might stumble upon this (though it seems unlikely) and might consider actually using it as the go-to negotiator for their site.

I also had one comment in the MR.

But overall this looks pretty good, so thanks again! Hopefully we can soon get it merged and get this issue finally fixed.