Remove LanguageNegotiationUrl's usage of base_path()

Unable to find base_path() in LanguageNegotiationUrlTest. Looks like its already fixed.

I'm a novice and I'm not sure what should I do if test passes on my local installation but fails during d.o test.

BTW, failed LanguageUILanguageNegotiationTest itself uses base_path()

replaced rtrim(base_path(), '/'); with $request->getBasePath();

hi,
hopefully this one passes :)

What about this !

don't give up !

#16 seems to make it but there is something wrong with the "// Test HTTPS via current URL scheme." in Drupal\language\Tests\LanguageUILanguageNegotiationTest
that's the only failing test

Last Patch does not apply anymore.

Something went wrong with reroll, the patch contains only an empty line now.

The failure in the tests can be traced back to the Request::create in LanguageUILanguageNegotiationTest::testLanguageDomain.

This only happens when running drupal from a sub-folder (something like http://example.com/drupal/), as far as I can see this is because the request::getBaseUrl method can't correctly generate a base url in Request::prepareBaseUrl.
I'm assuming the best way to fix this is to change the Request::create code to include one (or more) of the needed server variables (SCRIPT_NAME, SCRIPT_FILENAME, PHP_SELF) to make sure that the base url is getting returned correctly.

I can't seem to figure out what combination of those variables should work though.

Attached patch does remove the base_path function from the test. The LanguageNegotiationUrlTest still passes without this.

Maybe take a look at #2529170: [PP-1] Remove DrupalKernel::initializeRequestGlobals and replace base_root, base_url and base_path with a service, but I'm not sure whether the changes over there are isolatable and portable to this issue.

+++ b/core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationUrl.php
@@ -183,7 +183,7 @@ public function processOutbound($path, &$options = array(), Request $request = N
-        $options['base_url'] .= rtrim(base_path(), '/');
+        $options['base_url'] .= rtrim($request->getBaseUrl(), '/');

Two problems here.

The first one: Use getBasePath() (not getBaseUrl()). Also the Symfony version does not add a slash, therefore I expect the rtrim to be superflous.

The second one: $request->getBaseXXX() is always relative to the front controller (i.e. index.php). If this is executed from within a nested front controller (e.g. core/authorize.php), then /core is appended to the base path to the drupal root. This is what the other issue is trying to resolve.

Using getBasePath() over getBaseUrl() doesn't make a difference.
In the testing I've done, the request come from /index.php as front controller, so the change in the other issue doesn't seem relevant here.

I've added a patch that uses the getBasePath() and introduces 2 new debug statements so it's clearer what the actual problem is. The tests will still fail with the attached patch.

Postponing this issue until #2529170: [PP-1] Remove DrupalKernel::initializeRequestGlobals and replace base_root, base_url and base_path with a service lands.

This is included in the current patch in #2529170: [PP-1] Remove DrupalKernel::initializeRequestGlobals and replace base_root, base_url and base_path with a service. I think this can be closed.

No, the problem in this issue is NOT fixed by the patch in #2529170: [PP-1] Remove DrupalKernel::initializeRequestGlobals and replace base_root, base_url and base_path with a service. Maybe it should be, but none of the above information was posted to that other issue so the existing patch in that issue doesn't even try to remove base_path() from the unit test. Closing this issue just hides the problem and assures it won't be fixed.

This remains a valid, unaccomplished task that still needs to be addressed. If you think that #2529170: [PP-1] Remove DrupalKernel::initializeRequestGlobals and replace base_root, base_url and base_path with a service should handle this problem, then this information needs to be added to that issue and the patch in that issue needs to be re-rolled to include the changes from #36 above.

I just read the MR over in #2529170: [PP-1] Remove DrupalKernel::initializeRequestGlobals and replace base_root, base_url and base_path with a service and it now includes updating the Unit test. I am closing this as a duplicate and transferring credit.