The entity link label formatter does not check URL access.

Thank you for including test-only patch.

Only question would be the todo linking to follow up issues but will see if committers think that's an issue.

LGTM

@kksandr, thanks for issue and the MR!

I'm triaging RTBC issues → . I read the IS, which is missing several sections. I have restored those in case they are needed and so we have the 'remaining tasks' section. I read the one comment that gives thanks for the test only path and leaves a question for the committer team. I do not see evidence of a code review.

I applied the diff and read the comment with the @todo. The comment should be wrapped correctly and instead of referring to a file it should refer to a class, or method. That said, I then ran the test without the added user to see the failure. That was at /var/www/html/core/lib/Drupal/Core/ParamConverter/EntityConverter.php:149 which also has a todo. And that todo refers to https://www.drupal.org/node/2934192 → which is removing the block of code where the test is failing. It seems to me the @todo should point to that issue. Unless, I am missing something. Then, when that is fixed the creation of the user can be removed. (I tried that by applying that patch and removing the creation of the user and the test passed). Also, the anonymous user should only be created in the test where it is needed, instead instead of in the setup.

In testLabelFormatter, the value build[0]['#url'] is always unset. Surely, there should be a test for when it is set.

Setting to needs work for the above.

kksandr → closed merge request !4739

Seems all the todos have been removed/addressed

https://git.drupalcode.org/issue/drupal-3386313/-/jobs/1896646 shows the current test coverage is still there

The test chunk added in https://git.drupalcode.org/project/drupal/-/merge_requests/8447/diffs?co... believe addressed the test scenario where the URL is not unset.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Missed the additional threads after I marked RTBC but believe feedback has been addressed

One question on the MR.

Thanks that looks better to me.

Similar to the fix in 🐛 StringFormatter always displays links to entity even if the user in context does not have access Needs work this needs a change record to explain the behaviour change to users (and tests) that might be relying on the current behaviour.

It may feels copy-paste... because admittedly, it is! :)

The "Label" entity reference field formatter now restricts links for inaccessible destinations →

Thanks, the change record reads well and the code changes look good too.

Committed to 11.x
I think it is worth back-porting this to 10.4 for API compatibility and because its a bug.
I don't think we can back port it to 10.3/11.0 because the behaviour change could be disruptive.

Needs re-roll for 10.4.x

Published the change record.

Thanks

larowlan → closed merge request !8447

I backported this to 11.1.x

Setting to 10;5.x, but also needs backport for 10.4.x

Isn't there a legit use case for still linking to pages a user does not have access to? Like if the entity being linked to is not available to anonymous users, but is available to authenticated users. We'd still want the link to that entity because we can log the user in automatically when they encounter a 403 and are logged out.

Yes, you may want to suggest the user to log in because afterwards they **may** get access to something, but at the same time, you may not want to disclose information about the entity behind the scenes, like the title/label could already hold such information.

So in this cases you actually would like to expose a Call To Action based on a deliberate decision and driving the user.

So in this cases you actually would like to expose a Call To Action based on a deliberate decision and driving the user.

But I can't do that anymore with this change, right? At least, not using this field formatter.