Local tasks for node not visible on diff.revisions_diff route

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.

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.

I upload a patch if that helps.

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.

acbramley → changed the visibility of the branch 8.x-1.x to hidden.

acbramley → changed the visibility of the branch 2.x to hidden.

acbramley → changed the visibility of the branch 2954352-local-tasks-for to hidden.

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.