Entity rendering/theming does not use the active entity language to render links

langcode is language()->id. right?

Let's try again

Updating path alias test to work with this change...

Let's move forward with this for now. If there is a way (and time) to improve things later we will try :)

Re-roll because of EntityInterface::url().

drupal_2149649_20.patch no longer applies.

Re-roll.

.

Removing 'Needs reroll' as the latest patch apply without any conflict.

I took a look at the test changes, but I dont see where it is testing to make sure the title (or read more) link goes to the language the node is being shown in.

I tried it out, and it seems to help the title, but not the read more.

A
With head

on front page /af, all links (title and read more) go to /af/node/1

B
With patch

on front page /af, title goes to /af/node/1,
but read more goes to /node/1
which is not what I expected

C
with just #2006606: Views rendering ignores entity language, eg: Identical nodes rendered in views when nodes have translations (note I didn't run into the error that was last reported there, sent that issue for retest.)

on front page /af, all links (title and read more) (of the one showing en title and body, and of the one showing af title and body) goes to /af/node/1
which is kind of ok. the question is does this issue fix things once with the case of having that patch applied?

D
with patch and #2006606: Views rendering ignores entity language, eg: Identical nodes rendered in views when nodes have translations

on front page /af, it does fix the title link for the english displaying node to go to /node/1, but the read more links are both /af/node/1 (and when front page /, they are both /node/1)

------
So, I think this patch helps, but still needs work to make other links (read more) know the language also, and for tests to test it.

This is a major regression from 7.x, so bumping.

#2006606: Views rendering ignores entity language, eg: Identical nodes rendered in views when nodes have translations was committed, so we can just work on this directly. Right now, the read more links are what we need to get working.

just the readmore link tested.

copied from NodeAccessBaseTableTest
looked up the preg_match | syntax, but still not sure if it's needed. kept it in the copy and pasted code.

test only patch should fail. (fails locally)
patch should pass (the new tests pass locally)

---

while I was looking into this, I wondered why there is a $node->save(); after the hunk making translations. that is unrelated to this issue, but if it's not needed, would be a good novice issue to deal with it.

---
next, the comment (and remove the @var, which I was experimenting with)

adding tag back.

fail from #40 (see #2216701: Random test failure in ToolbarAdminMenuTest), but also

CollapsedDrupal\system\Tests\Common\RenderTest	137	6	0
Message	Group	Filename	Line	Function	Status
The token attribute was found in the cached child element markup	Other	RenderTest.php	909	Drupal\system\Tests\Common\RenderTest->testDrupalRenderChildElementRenderCachePlaceholder()	
The tokens are identical for the child element	Other 	RenderTest.php	914	Drupal\system\Tests\Common\RenderTest->testDrupalRenderChildElementRenderCachePlaceholder()	

and some similar ones.

retesting though, since the previous patch passed, and this one only changed a test.

43: drupal_2149649_42.patch queued for re-testing.

I thought of doing this, but the pattern for the default language (en in the test), is also present in the other langcode's hrefs, so it counts more than one.

    // Check the frontpage for 'Add new comment' links that include the
    // language.
    $comment_form_href = '/node/' . $node->id() . '#comment-form';
    foreach ($this->langcodes as $langcode) {
      $num_match_found = 0;
      if ($langcode == 'en') {
        // Site default language does not have langcode prefix in the URL.
        $expected_href = $comment_form_href;
      } else {
        $expected_href = '/' . $langcode . $comment_form_href;
      }
      $pattern = '|' . $expected_href . '$|';
      foreach ($this->xpath("//a[text()='Add new comment']") as $link) {
        if (preg_match($pattern, (string) $link['href'], $matches) == TRUE) {
          $num_match_found++;
        }
      }
      $this->assertTrue($num_match_found == 1, 'There should be 1 new comment link for the ' . $langcode . ' translation of a node on the frontpage. (Found ' . $num_match_found . '.)');
    }

so, went back to check checking it found at least one of each..

    // Check the frontpage for 'Add new comment' links that include the
    // language.
    $comment_form_href = '/node/' . $node->id() . '#comment-form';
    foreach ($this->langcodes as $langcode) {
      $match_found = FALSE;
      if ($langcode == 'en') {
        // Site default language does not have langcode prefix in the URL.
        $expected_href = $comment_form_href;
      } else {
        $expected_href = '/' . $langcode . $comment_form_href;
      }
      $pattern = '|' . $expected_href . '$|';
      foreach ($this->xpath("//a[text()='Add new comment']") as $link) {
        if (preg_match($pattern, (string) $link['href'], $matches) == TRUE) {
          $match_found = TRUE;
        }
      }
      $this->assertTrue($match_found, 'There is an Add new comment link for the ' . $langcode . ' translation of a node on the frontpage.');
    }

but that makes the check for en silly. (it wont fail if the en is not on the front page)

ah, on retest, #42 passes.

ok. @sun suggested using a[starts-with( in xpath. But since I wanted to check both the link text and the ref, used the xpath to get the matches to the link text and then preg_match to check the ref.

@jhodgdon mentioned that they might have written a function that checks for matches both the text and ref at one point for drupal 7, but I didn't get details of where that might be.

This uses base_path(). And makes sure there are exactly 1 matches.

I dont use the $matches variable that is passed to preg_match(), so maybe that should be taken out.

Also, The two hunks of code could be merged so we loop just once through the languages.

But those are kinda small things.

Putting this out there for people to check if it's even the right place for tests and going in the right direction.

Removing the needs tests tag since this adds tests.

Since people will ask why there are duplicates on the front page anyway, adding the related issues from the parent here as well.

the tests only failed as expected.

needs review. (we are very close, so look at everything, solution and tests)

I just added a related issue:
#2217569: Fields row plugin: Translation is non-uniform for base fields, Field UI fields, links; no way to choose "this row's language"
which has to do with field rendering in views (if instead of using "teasers" as the output, you select the "fields" row plugin and display the Body field for instance). Just pointing it out as people on this issue may be interested in the other one too...

Patch looks sane :)

The patch #50 cannot be applied to the latest 8.x any more.
I am going to reroll.

Reroll patch #50

Without the patch I experience the same problem as described in the issue summary.

I added 3 different languages (English, Hungarian, Polish)
I enabled the translation support for node:article on page admin/structure/types/manage/article.
And I enabled the translation support for some of the fields of the article on page admin/config/regional/content-language.
I created an Article with English language.
Translate to Hungarian
Translate to Polish, used the English version as source of the translation.
Both of them are promoted to front page. (Issue 2006606)
When I visit the front page the node title is points to the proper link, but the "read more" link is points to the current interface language version of the given node, even if such translation is not exists.

With the patch every "read more" link points to the proper language variants of the given node.

Thanks Sweetchuck for testing!

So it sounds like the patch now applies and has been manually tested to work.

I looked over the code and tests, and I think the code looks good (all 6 lines or so that changed), and the test changes also make sense.

In #50, there was also a test-only patch, which failed where it was expected to fail (I looked at the test result page, and the test definitely was finding the bug reported on this issue).

Wim in #55 also deemed the patch to be "sane".

The issue summary even reflects the current status of the issue.

So... I think this can be marked RTBC. Great work!

Committed 0a8e34c and pushed to 8.x. Thanks!

Enhanced tests to catch Node title link [$node->url()] errors.

@droplet can you open a new issue to do the enhanced testing - definitely looks like an improvement but it's not as critical as the original work.

#2234053: [Follow up] Enhanced Entity language tests

tagging needs followup per #66

I do not think this was fixed. It seems like the title links are still all going to node/1 not fr/node/1 or whatever. This issue was I think repurposed to only be about links. I'm reopening to see if anyone else agrees that it is not actually fixed.

Oh sorry, my mistake indeed! I was referring to Views. I'll see if there's an open issue...

The problem I am seeing is this:

If I make a view of nodes (which is really a view of node translations). I display it using Fields. I put in the Title field, and check the "Link this to the entity".

All of the links go to node/1, not to a translation page.

OK, I did a quick test and confirmed that if you use Content/display mode in Views, the links are correct. I think the links being wrong at field level are probably part of #2217569: Fields row plugin: Translation is non-uniform for base fields, Field UI fields, links; no way to choose "this row's language" and I'll make a note there. Thanks for the clarification!