Links from update_helpful_links() should point to site root and not to update.php

Re rolled the patch. This needs some attention. This should have been committed ages ago.

Change to be appropriate.

Yes.

If you run Update.php then the links will link back to update.php.

Hence why I tagged it as database update system.

Perfect, that is the design of url(), but shouldn't we use $base_path . '' ... i guess this is not possible?

$base_path returns "/" pretty much.

I think this is for the best. We now have tests to test for regression as well.

+++ b/core/update.phpundefined
@@ -166,14 +166,15 @@ function update_script_selection_form($form, &$form_state) {
+  global $base_url;

I think you might be abe to use Drupal::request()->getBaseUrl(); here instead of the global...

Marked #2020141: Wrong link to admin pages after succesful update as a duplicate... jeroen12345, suhel.rangnekar, marcoscano should be added to the commit message to ensure there work on producing exactly the same patch :) is credited.

Marking as RTBC.

Sorry... this isn't rtbc... we need to address the issue raised in #8

In collaboration with alexpott, we worked out that Drupal::request()->getBaseUrl() isn't working how we would like.

So for now it is best to use global $base_url;

This is due to Url() https://api.drupal.org/api/drupal/core!includes!common.inc/function/url/8

At the moment it isn't working properly, this comment is left in the code in common.inc

// Fallback to using globals.
// @todo Remove this once there is no code that calls url() when there is
//   no request.

alexpott will check on outstanding issues tomorrow with a few others.

+++ b/core/update.phpundefined
@@ -166,14 +166,15 @@ function update_script_selection_form($form, &$form_state) {
 function update_helpful_links() {
+  global $base_url;
   $links['front'] = array(
     'title' => t('Front page'),
-    'href' => '<front>',
+    'href' => $base_url,
   );
   if (user_access('access administration pages')) {
     $links['admin-pages'] = array(
       'title' => t('Administration pages'),
-      'href' => 'admin',
+      'href' => $base_url . '/admin',
     );
   }
   return $links;

I think what we can do this...

 function update_helpful_links() {
   $links['front'] = array(
     'title' => t('Front page'),
     'href' => '<front>',
     'absolute' => TRUE,
   );
   if (user_access('access administration pages')) {
     $links['admin-pages'] = array(
       'title' => t('Administration pages'),
       'href' => 'admin',
       'absolute' => TRUE,
     );
   }
   return $links;

That didn't work for me when I tried it.

Yep me neither... what is very interesting is my I access update.php the baseUrl property on the url_generator is set to http://my.drupal8.site/core when the actual base url is http://my.drupal8.site

I think we're dealing with a deeper issue in the routing system / url generation system.

Here's a fix that uses the same technique as we use in the installer - setting the script path and base path to use on the generator.

Works for me. Has my +1.

#17: drupal-2017769-17.patch queued for re-testing.

The issue here is deeper than just update.php

In UrlGenerator::generateFromPath we refer to $this->basePath which is set to be Symfony's $request->getBasePath() which returns the currently executing script's location. It looks like we're assuming that index.php will be the executing script.

So anytime we're not in index.php (e.g. install.php, update.php, authorize.php, etc.) this will return a different value.

Is it related to this issue? #2031353: URLgenerator broken for Drupal installed in a subdirectory - doesn't have a way to get a Drupal path

On that note, has anyone tested without drupal being installed in a subdirectory?

Marked #2029541: Links in update.php page don't point to the right location as duplicate, which has a similar patch to #17.

#22 - could you explain what you mean by "It looks like we're assuming that index.php will be the executing script."

I couldn't find mention of it in UrlGenerator::generateFromPath.

Sure, I'll elaborate.

In drupal 7 url() (the place where this code used to reside) the line reads

$base = $options['absolute'] ? $options['base_url'] . '/' : base_path();

Where base_path() is the global $base_path variable, which is the installation directory of drupal.

Now the line reads:

$base = $options['absolute'] ? $options['base_url'] : $this->basePath;

Where $this->basePath is set to be the directory of the currently executing script.

Perhaps this was done before scripts were moved to core/ and so the author didn't realize that $this->basePath would be different in different scripts.

I think probably the best solution is to make $this->basePath be based on the global $base_path variable again, and then change $this->scriptPath to include /core/ or whatever. Also, the comment on 'script' is misleading:

- 'script': Added to the URL between the base path and the path prefix. Defaults to empty string when clean URLs are in effect, and to 'index.php/' when they are not.

I think ideally, 'script' would just be a boolean indicating whether or not to include the script path since the path can be set using $this->setScriptPath()

Here's a patch just implementing the basePath change, and "fixing" the broken links on the update page. I'm kinda thinking more needs to be done, but I'm not sure if "API freeze" prevents this...

#26: base-url-2017769-26.patch queued for re-testing.

#26: base-url-2017769-26.patch queued for re-testing.

The "assumption" here, if there is one, is that we are using a front controller - update.php and install.php are not proper front controllers, and so they need to be special-cased. We certainly do not want to be putting calls to base_path() in the generator. The solution in #17 is what we need here: tell the generator what script path and base path to use when generating urls in update.php or install.php because it cannot get them from the current path as it is a stupid non-front-controller path.

+$generator->setBasePath(str_replace('/core', '', $request->getBasePath()) . '/');

What if there is another directory beginning with "core" in the path?

Fair enough. This gets my +1 too then.

As for #32, this is a special case, so we need not worry about other cases.

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

Marked #2044371: wrong links under update.php as duplicate.

@clemens.tolboom: #17 adds tests to ensure the links no longer contain "update.php"; if this is not enough, what else should be tested?

Oh, I see what you mean now :)

Attached is #26 plus tests from #17.

When I marked this as rtbc, I was referring to #17, not #26, sorry for the confusion. I'll attach it again just to make it clearer.

#38 - You posted an empty patch.

Someone needs a brain transplant (hint: the answer is me). here's an actual patch.

Nice

Confirmed the additions to the tests fail without the fix...

Committed 79347b8 and pushed to 8.x. Thanks!