Regression: "Display the file download URI" on related file entity shows relative path

This certainly seems a regression because of issue #2669074: Convert file_create_url() & file_url_transform_relative() to service, deprecate it →

As per the change record file_create_url()<code> is supposed to be moved to <code>\Drupal::service('file_url_generator')->generateAbsoluteString($uri)

Instead of that, it is updated with $this->fileUrlGenerator->generateString($uri)

That sounds the root cause of the regression.
Probably this wasn't caught becuase we don't have test coverage for this one.

I've raised a PR with the fix and working on test coverage.

Can the issue summary be updated to include the issue this was added? May have been done on purpose. 4 years makes me wonder if it was a regression

I checked the gigantic thread of the issue #2669074: Convert file_create_url() & file_url_transform_relative() to service, deprecate it →

The changes approximately starts from comment 83 https://www.drupal.org/project/drupal/issues/2669074#comment-13003933 →

I don't see any specific discussion of switching generateString instead of generateAbsoluteString to FileUriFormatter.

I believe this issue might not have caught till date because there might be very few sites who are using URI formatter.

I've updated issue summary as well. Happy to close if required.

Apologize I've had this tab open for like 3 days and keep getting distracted

Summary looks good now thanks for that!

Ran the test-only feature and got

1) Drupal\Tests\file\Kernel\Formatter\FileEntityFormatterTest::testFormatterFileUri
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'http://localhost/vfs://root/sites/simpletest/84724206/files/file.png'
+'/vfs://root/sites/simpletest/84724206/files/file.png'
/builds/issue/drupal-3253897/core/modules/file/tests/src/Kernel/Formatter/FileEntityFormatterTest.php:109
FAILURES!
Tests: 6, Assertions: 31, Failures: 1.

Wasn't 100% how to manually test but use of generateAbsoluteString() per the CR

Hi everyone,

I created a patch from MR and applied it to 10.5.1. Worked perfectly!

Pipeline is green. Test-only test fails as it should.

For #16, it's actually usually preferred to document the output of the failing test for the test-only in the job in a code tag, like so:

// Failed job output here

N methods failed Y asserts completed in blah blah

It's not sufficient that it fails; it needs to fail in the right way and then we document that. 🙂 Thanks!

#19 A rebase is required. Because I seem to upset people by doing rebases I didnt do it. No one triggered the test-only test. I read through the code changes. The test looks as tests go very simple and uses the same method as the code change. There seems little room for the test coverage to be inadequate, therefore. No one triggered the (manual) test-only test. I wanted to make sure the test failed so did more than just look correct. I also edited the IS.

Updated IS.