$entity->toUrl('revision-*') should fill revision parameter on all applicable routes.

I wonder if there is a reason the param isn't just added for all link templates? If the param doesn't exist in the URL, it would just be ignored right?

Reroll for 8.7.x

Re #6:

I wonder if there is a reason the param isn't just added for all link templates? If the param doesn't exist in the URL, it would just be ignored right?

I think this would be the best option.

Re #7

Just as an example, menu_link_content.module does this:

 foreach ($entity->uriRelationships() as $rel) {
    $url = $entity->toUrl($rel);
    // Delete all MenuLinkContent links that point to this entity route.
    $result = $menu_link_manager->loadLinksByRoute($url->getRouteName(), $url->getRouteParameters());

This wouldn't work correctly anymore, as loadLinksByRoute('entity.node.canonical', ['node' => 1, 'node_revision' => 1]) would not pick up the menu link for /node/1.

Unfortunately this is correct.

But what we can do is to return the parameter in \Drupal\Core\Entity\Entity::urlRouteParameters() and in \Drupal\Core\Entity\Entity::toUrl() pass it to the URL object only in case this parameter is present on the route. What about something like this:

Make \Drupal\Core\Routing\RouteMatch::getParameterNames() somehow accessible -

1. make it static public and add a parameter for the route object
2. make it public
3. copy-paste its logic here

An approach based on the 3rd option:

$route = \Drupal::service('router.route_provider')->getRouteByName($route_name);
// Variables defined in path and host patterns are route parameters.
$variables = $route->compile()->getVariables();
$names = array_combine($variables, $variables);
// Route defaults that do not start with a leading "_" are also
// parameters, even if they are not included in path or host patterns.
foreach ($route->getDefaults() as $name => $value) {
  if (!isset($names[$name]) && substr($name, 0, 1) !== '_') {
    $names[$name] = $name;
  }
}

Now that we know the necessary parameters we can remove the unnecessary ones:

$route_parameters = $this->urlRouteParameters($rel);
$route_parameters = array_intersect_key($route_parameters, $names);

Reroll since #1883744: Convert Entity to EntityBase for consistency with ContentEntityBase and ConfigEntityBase

Setting back to Needs Review & Triggering bots.

Manually tested #15 while working on #2350939-77: Implement a generic revision UI today, and it works as expxcted, tested using the link template revision-revert-form on the version_history route, using the block_content Entity Type.

This is now an additional blocker for the issue I just linked.

To be honest I think #12 is too complex if we can fix the actual issue with a patch as simple as #5 / #11 / #15. I agree with everyone here that the current API around this is super weird but it is the API we have. And #12 for example wouldn't work with "extra" route parameters which do not belong to variables, such as the entity type ID on the collection route. So I think #15 (or something like it is the way to go).

This is rightly marked "Needs tests", however, so marking "needs work" for that.

+++ b/core/tests/Drupal/Tests/Core/Entity/EntityUrlTest.php
@@ -176,11 +176,11 @@ public function testToUrlLinkTemplateRevision($is_default_revision, $link_templa
+    $url = $entity->toUrl($link_template !== 'canonical' ? $link_template : 'revision');

I think this would be more understandable if we move $link_template out of the method call and reassign it:
$link_template = $link_template === 'canonical' ? 'revision' : $link_template;

Feel free to set to RTBC when done :).

--- a/core/lib/Drupal/Core/Entity/EntityBase.php
+++ b/core/lib/Drupal/Core/Entity/EntityBase.php
@@ -271,7 +271,7 @@ protected function urlRouteParameters($rel) {
       $parameter_name = $this->getEntityType()->getBundleEntityType() ?: $this->getEntityType()->getKey('bundle');
       $uri_route_parameters[$parameter_name] = $this->bundle();
     }
-    if ($rel === 'revision' && $this instanceof RevisionableInterface) {
+    if ($this instanceof RevisionableInterface && strpos($rel, 'revision') === 0) {
       $uri_route_parameters[$this->getEntityTypeId() . '_revision'] = $this->getRevisionId();
     }

Perhaps adding an inline comment here would be beneficial so that it's easy to see what the impact of this is. I'm also sort of split on whether strpos is fine or whether substr would be more readable here.

I think that the code is self explanatory and does not need any additional documentation. Even if we add it I do not see the added value of documenting a simple condition.

Adding review credit for @AaronMcHale for manual testing and @hchonov for subsystem-maintainer review

Committed 8350f68 and pushed to 9.1.x. Thanks!

Updating some items this blocks

Thanks @larowlan, great to see this small yet important patch land, well done everyone!

I add to this issue the same patch, compatible with version 8.9.7 of the Drupal core.