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

Quick little git-blame points to #2669074: Convert file_create_url() & file_url_transform_relative() to service, deprecate it, so that would not make this due to a change in Views but on the field formatter.

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!

Test-only test output:

Time: 00:15.009, Memory: 8.00 MB
There was 1 failure:
1) Drupal\Tests\file\Kernel\Formatter\FileEntityFormatterTest::testFormatterFileUri
Failed asserting that two strings are equal.

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.

But that has already been done for this issue, so there is no need to repeat it after RTBC. 🙂

I guess you were trying to say your rebase had the same test-only fail as in the previous comment?

RE: #19

I guess you were trying to say your rebase had the same test-only fail as in the previous comment?

Actually, i think that makes sense. Edited #16. When I read #13 for some reason I got the impression @smustgrave was reporting a problem with the test.

Updated IS.

Committed/pushed to 11.x, cherry-picked to 11.2.x and 10.6.x thanks!