url() should return / when asked for the URL of the frontpage

This is definitely not optimal. Triaging.

The gist of this issue is that url() should return / when asked for the URL of a path that is assigned to the frontpage.

There is nothing wrong with relative canonical links. Drupal's default behavior is to output relative links everywhere (this behavior can be altered using hook_url_outbound_alter(). There is also nothing wrong with the "shortlink" being potentially longer then the "canonical" either. The original specification mentions "[The shortlink] will typically be shorter than other URIs".

I agree url() should return / for <front>.

#1: I agree the canonical link can be relative. But shouldn't the shortlink be an absolute URL?

+1

Issue are fixed in d8 first

fixed invisible tag

In D8 this is relevant when using \Drupal::service('path.alias_manager')->getAliasByPath('/node/' . $node->id());, returns '/node/[nid]' instead of '/' when the node is set as the front page.

We fixed this in a client project by overwriting the UrlGenerator class's doGenerate method with this:

$url = parent::doGenerate($variables, $defaults, $tokens, $parameters, $query_params, $name);

// Load the front page config.
$frontPagePath = \Drupal::config('system.site')
  ->get('page.front');

// If the url currently being generated is to the front page, overwrite the
// output to /.
if ($frontPagePath == $url) {
  $url = '/';
}

return $url;

@borisson_ It seems that your solution fix automatically a lot of problems relatives to the frontpage url case issued in various modules (alternate hreflang url, canonical url, frontpage urls displayed in various places (view content, ...)). But, it's a bit too brutal, you can't access anymore to the internal path of the current object if it match as the frontpage and so it breaks a lot of things.

Stuff like:

Url::fromRouteMatch(\Drupal::routeMatch())->getInternalPath();
\Drupal::service('path.matcher')->isFrontPage();

Become ineffective for the object/entity/view/whatever defined as frontpage.

At what level should it be fixed? In UrlGenerator OR Entity->toUrl() OR ... ? I can't see a kind of global solution for this right now.

See my comment #2987635-4: Front Page canonical is Incorrect

I guess the frontpage canonical url being not the internal path or aliased path, is the most expected behaviour by most of the people, no?

If a new kind of rel canonical ('canonical-aware') or a new option for Entity->toUrl() ('frontpage-aware') have to be created, all core and contribs modules should update their way to retrieve the URL of an entity following the context.
Or there is just no real global solution possible in the current state of the core and it should stay the responsibility of each developer/module to manage the frontpage url case when needed?

In case anyone is looking, I created a service implementing the OutboundPathProcessorInterface to return '/' for anyone generating a link to the front-page node ID. What I don't understand is why PathProcessorFront.php in core/lib/Drupal/Core/PathProcessor does not do this, but only replaces `/` with `/`.

my_module.services.yml

services:
  my_module.path_processor:
    class: Drupal\my_module\FixHomeUrlAliasOutboundPathProcessor
    tags:
      - { name: path_processor_outbound, priority: 400 }
    arguments: ['@config.factory']

FixHomeUrlAliasOutboundPathProcessor.php

<?php

namespace Drupal\my_module;

use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\PathProcessor\OutboundPathProcessorInterface;
use Drupal\Core\Render\BubbleableMetadata;
use Symfony\Component\HttpFoundation\Request;

/**
 * Outbound path processor to fix home aliases.
 *
 * This needs to fire BEFORE the alias processor.
 */
class FixHomeUrlAliasOutboundPathProcessor implements OutboundPathProcessorInterface {

  /**
   * Constructs a FixHomeUrlAliasOutboundPathProcessor object.
   *
   * @param \Drupal\Core\Config\ConfigFactoryInterface $config
   *   A config factory for retrieving the site front page configuration.
   */
  public function __construct(ConfigFactoryInterface $config) {
    $this->config = $config;
  }

  /**
   * {@inheritdoc}
   */
  public function processOutbound($path, &$options = [], Request $request = NULL, BubbleableMetadata $bubbleable_metadata = NULL) {
    // Get the route for the front page.
    $front_path = $this->config->get('system.site')->get('page.front');

    // There is no front page, so bail.
    if (empty($front_path)) {
      return $path;
    }

    if ($front_path === $path) {
      return '/';
    }
    else {
      return $path;
    }
  }

}

Have you tried entering "/" as the path alias for that node?

I have tried adding / as an alias to the homepage - the simple xml sitemap now generates two urls for the homepage:

https://www.example.com/

and

https://www.example.com

The second one should not be there at all?

I made a patch for PathProcessorFront based on #14. I added some extra logic, as the path configured for frontpage can also be an alias. I did some testing and so far seems to works well. Besides further testing, we probably need to add unit test as well. I have no experience here and it's seems a little more complicated, maybe someone else iabel to provide ii?

This patch may be more reliable on older Drupal core versions.

The patch from #20 fails, because it reintroduces deprecated namespaces.

The path language must be considered as it can differ from the current language.

The code style is fixed.

#18 worked for me

#20 worked ... :)

Path Alias core module is optional and hence probably we cannot directly inject this service. We will have to find some alternative solution.

Checkout this: https://www.drupal.org/project/drupal/issues/3096092

With this patch applied on drupal/core, Drupal installation fails.

Temporary Fix to use path_alias.manager conditionally, created patch.

Somehow hasService is working differnetly, I've updated the temporary solution to use Drupal module handler.

Patch 25 modified to support drupal 9.2.x Not working as path.alias service can't be used directly.

Nilesh Chhatbar Can u send to me this files after fix? something i do wrong and i have still problem after fix, site take down
or public paste surc both files - i don't use git to update- manualy edit

Attached is a path that applies to 9.3.x. Drupal\Core\PathProcessor\PathProcessorFront lost its OutboundPathProcessorInterface implementation in 9.3.

Just fixed the custom command failure error in #37, thanks!

So how about if the logic was moved into the path_alias module?

I missed a bit in the last patch.

Resolves CSpell error & added an interdiff.

The tests failures are down from 97 to 15, that's an accomplishment ;-)

Just postponed #3100350: Unable to save '/' root path alias on this one so triaging it some.

Cleanup the issue summary would help. Started with the template.

Will also need tests.

Thanks @smustgrave. So @yogeshmpawar could you perhaps reroll this as MR and try to fix the remaining tests?

I created an issue fork on 11.x and applied patch "1255092-41.patch" by @yogeshmpawar.

Hm, Jenkins seems to be configured differently for 11.x? At least 1 PHPStan error, doesn't really make sense.

Changing to 10.1.x-dev

@Grevil: I think we should still target 10.1.x - if needed, we can switch later.

Alright, no chance to run the tests remotely on neither 10.1.x nor 11.x. I'll test it locally...

The patch had a breaking change in the constructor parameters, which of course isn't allowed, at least if it's not consistently changed.

So for now I reverted that change, but kept the class property:

  public function __construct(AliasManagerInterface $alias_manager) {
    $this->aliasManager = $alias_manager;
    $this->config = \Drupal::service('config.factory');
  }

Unsure what's the DI strategy here in core for this case, perhaps someone else could have a look?

I guess the tests should run now.

Believe should be targeting 11.x as that’s the latest development branch. And changes can be backported

@Grevil did you read #56?

Any good reasons why you broke it again later in https://git.drupalcode.org/project/drupal/-/merge_requests/4030/diffs?co...?

I don't think it's a good idea here to provide the generic config as 2nd parameter in __construct() in this class where there's no DI.
Where ever this is called outside, that call would have to be replaced to provide the 2nd parameter. Isn't that a BC here then?

Of course I might be wrong, but please leave a comment here then to explain the reasons.

You also revertet my other fixes without leaving a comment. Why?

Now we're back at the state where we were before with issues like

Running PHPStan on *all* files.
 ------ -------------------------------------------------------------------- 
  Line   core/modules/path_alias/src/PathProcessor/AliasPathProcessor.php    
 ------ -------------------------------------------------------------------- 
  71     Variable $system_config in empty() always exists and is not falsy.  
 ------ -------------------------------------------------------------------- 

Since we are adding dependencies to an existing interface, I guess we need a change record as this is a breaking change.

@Anybody, the class you manually implemented the "\Drupal::service" call does indeed not implement the "ContainerInjectionInterface". But that's because it is a service itself and the dependency injection happens through the service.yml (see "core/modules/path_alias/path_alias.services.yml" -> "path_alias.path_processor").

From the MR by @Anybody:

Are we sure the $path is passed as /<front> indeed?
I didn't check that, but in the UI you typically use <front> without leading slash, so this should be double-checked!
Perhaps write a test for this to be 100% sure, if not yet existing.

This is definitely important as "/" is quite untypical!

I don't have much time the next couple of weeks, so if anyone could add a test for this and do some general clean-up, go for it!

Did not test

Issue summary needs updating
Change record
Release notes

Change record drafted: https://www.drupal.org/node/3362769
Release note added to the issue summary.

Still needs work for the CCF.

Will leave it at needs work for the tests and issue summary updates.

I think this should not be in the path_alias module, which is now technically optional although most people are using it. It should be a separate event subscriber in core.

Solution:

1. Install and enable the "Metatag" module.
2. Configure the Metatag module by navigating to admin/config/search/metatag.
3. Define a custom token for the canonical URL by clicking on "Add custom token" in the Metatag configuration.
4. Set the custom token as [node:url:absolute] for the canonical URL on the front page.
5. Save the configuration and clear the Drupal caches.

By following these steps, the Metatag module will allow you to define a custom token for the canonical URL. You can set this custom token as [node:URL: absolute] for the canonical URL on the front page. Saving the configuration and clearing the Drupal caches will ensure that the updated canonical URL settings take effect.

@DSushmita thanks for the workaround, but at least for the cases discussed here, #67 isn't a real solution. Still, it might help some folks.

What's everyone's opinion on @berdir's comment in #66? What would be a good place for this Event Subscriber, if we would move the implementation to core?

Another related one: #2421941: Determine whether not only "/" but also "<front>" should be valid input for the Link widget

Looks like duplicate but not really #3394691: setting path alias to `/` causes PHP8.1/str_starts_with/null error

Another approach made to this in branch 1255092-front_page_outbound_url_path_processor, based as well in URL processor but outside of URL alias as suggested by Berdir at #66. I added outbound URL processing to the existing front page path processor.

It may fail in case of a front page path with query parameters were set, but it is also not well managed by other core parts like PathMatcher, see #3442235: Front page path with query parameters makes front path condition to fail. Added as related issue since support for query parameters should be added.

Will need test coverage and issue summary update.

Left some comments.

I'm asking myself if this could be fixed in contrib until it's solved in core one day.

Perhaps using OutboundPathProcessorInterface (https://www.drupal.org/node/2238759)?
Any ideas? What do you think?

Rebased the issue branch against 11.x and added a test :)

Sorry, I forgot to add a "clear caches" in the steps to reproduce.

Three core tests are failing, and despite spending some time investigating, I haven’t been able to resolve it.

• Drupal\Tests\system\Functional\System\ThemeTest::testThemeSettingsRenderCacheClear
• Drupal\Tests\big_pipe\Functional\BigPipeTest::testNoJsDetection
• Drupal\Tests\path_alias\Functional\UrlAlterFunctionalTest::testUrlAlter

Hiding older MR since it is no longer relevant as per #1255092-66: url() should return / when asked for the URL of the frontpage. Path alias module is optional.

@berdir in #66 you wrote

I think this should not be in the path_alias module, which is now technically optional although most people are using it. It should be a separate event subscriber in core.

and in #69 @grevil asked:

What's everyone's opinion on @berdir's comment in #66? What would be a good place for this Event Subscriber, if we would move the implementation to core?

We're running into this again and would finally like to fix it in core, but it would be sad if it goes into the wrong direction.

Do you have a specific idea where this should be implemented cleanly? In #77 I thought about OutboundPathProcessorInterface. If you could give us a clear direction, @grevil could try to solve this tomorrow. Feel free to contact him in Slack or write here if you (or any other Core-expert) has some minutes.

Thank you so much! Hopefully this is a good chance to get this DONE :)

PS: Just saw that your comment was before the latest MR that started fresh in #74 that doesn't use the path_alias module anymore and instead uses OutboundPathProcessorInterface and others, which looks good! Maybe the comment is already outdated, we just need to finish MR!7619 (and its tests)? That would be lovely.

Ok, I found the reason why the tests fail.

Inside most Functional, JS Functional and Nightwatch tests, the front page is configured as "user/login". Now if we use Methods like "Url::fromRoute('user.login')" we won't get "user/login" returned anymore, but instead we'll get "/".

IMO, this is fine and expected, but of course all the tests need adjustments to actually succeed now.

We have several ways to fix this issue:

• Define another front page on setup for all failing test classes. (Preferred)
• Globally, define a different front page.
• Adjust the tests to use other urls than "user/login" to test their functionality.

Diff from MR does not apply with recent Drupal core 11.3.3 anymore. On 11.3.2 patch applied. MR needs rebase.