Duplicate "revision_history" local tasks created

GitLab comment added.

Could we add a test case that checks the tabs for any duplicates?

This includes custom entities, contrib entities, etc that already had them added themself and results in duplicates.

I'm a bit confused by this because the version_history route didn't exist before ✨ Implement a generic revision UI Fixed was committed. It would be good to get some steps to reproduce here to show how duplicates are generated.

Example from Group:

# From group.links.task.yml
group.version_history:
  title: 'Revisions'
  base_route: 'entity.group.canonical'
  route_name: 'entity.group.version_history'
  weight: 20

And \Drupal\group\Entity\Group

 *     "route_provider" = {
 *       "html" = "Drupal\group\Entity\Routing\GroupRouteProvider",
 *       "revision" = "\Drupal\entity\Routing\RevisionRouteProvider",
 *     },

So any module that was using https://www.drupal.org/project/entity → to already get revision goodies before core started supporting it is now seeing the version history show up twice. This is because core has:

entity.version_history:
  title: 'Revisions'
  weight: 20
  deriver: 'Drupal\Core\Entity\Plugin\Derivative\VersionHistoryLocalTasks'

Which will trigger even if you're not using \Drupal\Core\Entity\Routing\RevisionHtmlRouteProvider.

In short: With Entity API (the module), people had to manually declare their local task, now core declares it a second time even if you're not using the revision routes provided by core.

My suggested solution: In D10 only supply the local task if the entity class is using core's route provider or a subclass thereof. In D11, we remove that check and rely on contrib to remove their copy of the local task declaration.

Actually, entity.module also had some features here, which I'm trying to remove over in 🐛 Remove duplicate version history local tasks (10.1+) Needs review . But we should still make the world a better place by trying to remove duplicates.

I think, we could also remove the hook node_local_tasks_alter here.
see: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/node/...

I'm still not sure this is core's responsibility. It seems like the Entity module should handle this instead, which it looks like it is over in
https://git.drupalcode.org/project/entity/-/merge_requests/28/diffs

I'm confused by this:

In short: With Entity API (the module), people had to manually declare their local task

Because the MR above would suggest otherwise.

Re #9 - that's something different and requires Node to use the generic revision UI, see the comment in that hook.

Not all custom code uses contrib entity module. The place that gets the greatest visibility is in core.

Not all custom code uses contrib entity module. The place that gets the greatest visibility is in core.

Yes but it's not core's responsibility to take into account what someone MIGHT have done in custom code or contrib (should we also detect duplicates for every single other derived local task, menu item, route, etc that core provides?).

We've got the fix for entity API module, why did Group module implement it manually again? It seems to me like it's a much better fit for Group (and others) to do their own logic on whether to add the local task or not just like Entity module.