Local tasks for node not visible on diff.revisions_diff route

The patch #2 worked for me on Drupal 9.4.5, seems like a simpler solution compared to #3169729: No local tasks on compare revisions page

Added also tests to make sure the local tabs are visible.

Patch #5 worked for my Drupal 9.5.11 site.
I added the following to the composer.json file
"patches": {
"drupal/diff": {
Displays local tasks on revisions sub-tab: "https://www.drupal.org/files/issues/2023-10-05/2954352-diff-show-local-t..."
}
}

+1 for RTBC

Great and simple improvement. Thanks!

Unfortunately this has caused issues on Node revision pages when there is more than one secondary tab (which is what is required to actually render the link)

Symfony\Component\Routing\Exception\MissingMandatoryParametersException: Some mandatory parameters are missing ("left_revision", "right_revision", "filter") to generate a URL for route "diff.revisions_diff". in Drupal\Core\Routing\UrlGenerator->doGenerate() (line 187 of core/lib/Drupal/Core/Routing/UrlGenerator.php). 

@acbramley could you provide a bit more information about the issue you have faced? I'm not being able to reproduce it on route diff.revisions_diff eg. /node/1/revisions, when selecting also "Show primary tabs" on "Secondary tabs" blocks.

@sokru you need to have at least 1 other secondary tab to trigger it.

This patch is a port of the patch in #3169729: No local tasks on compare revisions page. It should avoid the problem mentioned in #12.

Please use a merge request, patch files are not accepted on this project anymore.

Merge request with patch #17.

There are also other changes to get phpstan to pass again.

I'm using 2.0.0-beta3 and the changes in MR 64 work perfectly for me and solved a client request. Thank you!

So RTBC from that perspective, but I see that this MR targets the 8.x branch, so I'll refrain from updating the issue Status for now.

Rebased on 2.x.

This needs tests. I'd recommend adding tests from https://www.drupal.org/files/issues/2023-10-05/2954352-diff-show-local-t...
and separate test for issue reported on #12.

I have a commit ready to add the tests, but I don't seem to have access to the MR.

I'm not sure we need a test for the issue reported in #12 since the new approach doesn't have that issue, and from my experience of trying to do it the wrong way too (before I found this issue), the fact that we can load the node edit page earlier in the test proves there isn't an issue.

EDIT: Not sure why I didn't see the get push access button before, but I found it and was able to push my commit.

This still needs work for issue mentioned on #12 (and #16). So it needs steps to reproduce, fix for that and tests to make sure it won't get regression.

The merge request avoids the issue in #12 and it has a test.

I've tested the new approach and while it does fix the issue in #12 I'm not sure the performance overhead of this is something I'd be keen to commit to.

I debugged through and that alter hook is run on every single page load of a revision comparison page. None of the static caching for the tasks or route information works since we're not on the canonical route which means it's doing a lot of work to get those tasks, we also don't have test coverage for the bug in #12.

To reproduce it, you just need another secondary task under Revisions, e.g we have this so the secondary tabs render when on a Revisions route.

entity.node.version_history_sub:
  route_name: entity.node.version_history
  parent_id: entity.node.version_history
  title: 'Revisions'

Core has a similar pattern with Content moderation to render an Overview secondary tab on admin/content so the Moderated content tab shows.

re-roll for 2.1.1

I tested the re-roll from comment #34 (diff-add_local_tasks-2954352-34.patch) against diff 2.1.1 on a Drupal 10.6.14 site with around 200 contrib modules.

The patch applies cleanly without any additional re-roll, and everything is working as expected. Local tasks (View, Edit, Revisions, etc.) now show up correctly on the node revision compare page, matching the tabs available on the node's canonical page. "Revisions" is also correctly marked as the active tab.

Regarding the concerns raised in #32, I think they're valid and we agree with them.

* Performance: hook_menu_local_tasks_alter() does bypass the local task caching, since it runs on every page load of the revisions diff route instead of benefiting from the caching used by the canonical route.

* Regression from #12: We also looked into the alternative suggested by the maintainer: adding a declarative *.links.task.yml entry with base_route: entity.node.canonical, similar to what content_moderation.links.task.yml does. Unfortunately, this doesn't really solve the problem; it just moves it. Registering the diff compare route as a sibling task under entity.node.canonical causes it to be included in the same tab group on all node pages (View, Edit, etc.). However, the diff route requires left_revision, right_revision, and filter parameters, which aren't available on those pages. This brings back the same MissingMandatoryParametersException from #12, just in the opposite direction.

A proper declarative solution would probably need a custom local task plugin that can detect when those parameters aren't available and simply omit itself. That's more involved than just adding a YAML entry.

With that in mind, for sites that don't have another local task registered under the revisions route (which is the specific case that triggers #12), I think this patch is a reasonable and low-risk stopgap.

We're using the patch as-is rather than maintaining a custom fork of the logic, mainly so we can stay aligned with whatever solution this issue eventually settles on.

Happy to help test a proper declarative implementation if someone decides to work on one.