Add useful local tasks tabs when viewing a node revision

I applied the patch from #2730631: Upcast node and node_revision parameters of node revision routes, and it does not seem to fix this issue.

This patch adds the local task back in and solves the problem of missing route parameters by removing it in a hook_menu_link_alter on other routes.

It depends on the patch that upcasts the node_revision parameter: https://www.drupal.org/files/issues/2730631-35.patch

Updated patch that fixes the "no newline" message.

Note: patch depends on #2730631: Node revision routes don't convert {node} param

I realized I don't need to lookup the route.

I updated the patch, I realized there was a more efficient way to do it without a for loop.

Note: patch depends on #2730631: Node revision routes don't convert {node} param

I got #11 working, thank you. My feedback:

1. It adds a new local task "View Revision", which is only visible on the revision page. When navigating to a different local task (such as "Edit"), the new "View Revision" local task is not visible. This feels off, local tasks disappearing when navigating between local tasks.
2. I personally expected the "Revisions" local task to be active, which is not the case. Or is this a wrong expectation?

Before arriving at this ticket, I already had worked on an alternative solution. Instead of creating a new local task, I just use the node canonical local tasks on the node revision page. I also make sure that the revision history ("Revisions" local task) is marked active. Unsure if this is the way forward, but it does seem to work. See the patch attached. (FYI: this does *not* depend on the patch in #2730631: Upcast node and node_revision parameters of node revision routes).

@rp7: thank you very much !

Just in case people don't realize this...

You don't have to patch core to use this solution. You can take the code from the patch which is just a hook implementations and add it to a custom module. May be able to do this in the theme file but I didn't test it.

In my case, i have a custom module that fixes #2730631: Upcast node and node_revision parameters of node revision routes (See comment #18 of this issue). I just added the hook to this module's .module file and viola.. I don't have problems with code that gets the current node and I have tabs on revisions.

Thanks to all who have debugged this issue.

This builds off of #15. In addition to adding hook_menu_local_tasks_alter() to add the the local tasks, I also modified node_is_page() to also check node revision routes to ensure they display properly.

+1 for the patch in #21 - works as expected and is exactly what I needed

Applied patch #21 on 9.5.x and it works fine.

Before Patch:

After Patch:

RTBC +1

Verified and tested #21 patch applied successfully and looks good to me
Testing steps-
1. Install Drupal Standard profile
2. Set Claro as the default theme.
3. Create basic page content item and save
4. Edit same page node, create new revision
5. Go to node's revisions tab, node/1/revisions
6. Click the initial node revision to view it, /node/1/revisions/1/view
7. Observe no tabs present on revision path

Testing Results-
Node local tasks tabs appears on node revisions after applying the patch
Can be move to RTBC

+++ b/core/modules/node/node.module
@@ -32,6 +32,8 @@
 
 /**
  * Implements hook_help().
@@ -371,10 +373,18 @@ function node_revision_delete($revision_id) {

@@ -371,10 +373,18 @@ function node_revision_delete($revision_id) {
  */
 function node_is_page(NodeInterface $node) {
   $route_match = \Drupal::routeMatch();
-  if ($route_match->getRouteName() == 'entity.node.canonical') {
+  if ($route_match->getRouteName() == 'entity.node.canonical' || $route_match->getRouteName() == 'entity.node.revision') {
     $page_node = $route_match->getParameter('node');

node_is_page() deliberately only returns TRUE on entity.node.canonical - I don't think we can just change the behaviour here.

It might be necessary to refactor whatever logic is calling node_is_page() to do check something else (like just the full view mode?) and deprecate the function instead.

Also needs test coverage.

Other questions:

- what happens if someone has already added tabs on the node revisions page?

- is it OK to show, for example, the edit tab here?
- this marks the 'Revisions' tab as active, but the revisions tab is a list of revisions, not an individual revision, so it's misinforming the user where they are on the site.

To be honest I think leaving things how they are with no tabs but a breadcrumb back to the canonical node page is fine, but leaving open for opinions.

Discussed this briefly in bugsmash with @smustgrave and @darvanen and between us we agreed just copying the node local tasks over is going to be confusing for people.

I think we should consider repurposing this to add explicit local tasks to the revisions page if there are ones that are needed.

I agree with @catch.

I can think of a few things I would want to see from a revision view:

• View
• Revert to this revision
• Latest revision
• Revisions

Where 'view' is the active tab, i.e. the revision being viewed, and could potentially be labelled in a similar way to how they're shown in the revisions tab, e.g. "26/07/2022 - 10:40 by admin". Maybe just the date/time portion, idk.

@andrewmriv Thank you for your patch, did exactly what I needed :) And you fixed the double title thing too - perfection!

Thanks. Fixes missing tabs for me as well.. and does also fix our double title issue (which i had fixed with a preprocess function).

Had a quick look at how Google docs and Gitlab wiki pages do this. Both explicitly *remove* the UI for editing when showing an older revision of a document.

Google docs adds a primary button to revert to this revision

Gitlab wiki provides links to the current revision and the revision history overview

So I tend to agree with @29 in that we should not add the standard set of node local actions. Adding the option to directly restore to the currently shown revision seems useful. How's this for a new title for this issue?

I've re-rolled the patch for Drupal 10.1

Rerolled for 9.5.x.

Moving to NW for the test

Thanks!

Based on the code in this patch, I've implemented a revision pager of sorts as secondary tasks:

I don't really see a problem with keeping the node canonical tabs in place if revision-related tasks are always secondary. I'm considering adding a "Revert" option to this.

I originally had something like this way back in Drupal 6 and remember my editors liking it. The revision label changes to "Latest revision (xx)" or "Published revision (xx)" when on those revisions.

But maybe making navigating between revisions easier is something worth considering here? It's easy to get lost if you have to keep going back to the full revision list.

Verified and tested #38 patch applied successfully and resolved.

Steps to reproduce
1. Install Drupal Standard profile
2. Set Claro as the default theme.
3. Create basic page content item and save
4. Edit same page node, create new revision
5. Go to node's revisions tab, node/1/revisions
6. Click the initial node revision to view it, /node/1/revisions/1/view
7. Observe no tabs present on revision path

For reference:
Before patch

After patch

Results:
After applying the patch, the Node Local Tasks tabs are visible on node revisions.
RTBC ++

Please please read the tags and issue summary, this was tagged for tests which are still needed.

@smustgrave My apologies. I will adhere to tags moving forward.

I can confirm that the patch in https://www.drupal.org/project/drupal/issues/2885278#comment-15144519 works for Drupal 10.1.6 version.

I cannot apply patch #37 or #38 to the fork for Drupal 11.x. A reroll is required

I have added commits to the issue fork. I have used an existing functional test function but added assertions. There is now a failing test that will hopefully pass once the the patch(es) are applied.

I tried re-rolling both patches at #37 and #38 without success. It would be good if someone who has worked on the fix could add commits to the issue fork to see if they can make the test pass.

Changing the status since the functional test assertions are passing in the pipeline following #50.

I'm not sure why but the MR does not contain the new test assertions. They were added to the fork in an old commit. Will need to add them once again to prove that the 'fix' makes the test pass.

Not sure if phpunit has been upgraded but my local phpunit has stopped working. Seems the test assertions for this issue are also not running in the pipeline. The code is added to the MR and the MR should be ready to review (if phpunit is working).

Testing the MR via the browser I am seeing this: "fatal error: Cannot use Drupal\Core\Cache\CacheableMetadata as CacheableMetadata because the name is already in use in /var/www/html/core/modules/node/node.module on line 38"

There is also a test failure in the pipeline:

"1) Drupal\Tests\node\Functional\NodeRevisionsUiTest::testNodeRevisionsTabWithDefaultRevision
Behat\Mink\Exception\ElementNotFoundException: Element matching xpath "//ul[@class="tabs primary"]/li[@class="tabs__tab is-active"]/a[@href="node/1/revisions"]" not found."

First, I fixed a phpstan issue in the MR so that the pipeline shows green. After that, created a new FunctionalJavascript test using the Claro (admin) theme. It is passing with the MR applied so changing the status to 'Needs Review'.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Applied bot fix.

Left comment on MR.

Test is moved to the node module.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

After running into rebasing problems, created a new 11.x branch and added the patches/ MR's to it.

Several tests need to be fixed.

I have committed the changes to fix the MR, now the failing tests are fixed. Please check

Left comments.

Thank you @ramprassad, the tests seem fixed now: the pipeline runs all green except for the test-only test. It fails like it should.

That seems to leave several out-of-scope typehints @smustgrave has identified that need to be removed from the MR.

May need to think approach as not seeing why we are adding comment hooks into node. Should try and keep things separate if possible.

I went through the MR and removed the type hinting and the node hooks as per the MR review.

Hooks will need to be converted to OOP functions

Marking as needs review since I updated the merge request to use a hook object instead of a regular hook.

I wasn't able to apply the patch on 10.4.8, so I re-applied the changes here.

https://www.drupal.org/files/issues/2025-06-10/2885278-add-local-tasks-t...

Left comments on MR

Moved the config translation hook over to the NodeHooks file and removed the additional NodeLocalTaskHooks.
Fixed some lint errors.

https://git.drupalcode.org/project/drupal/-/merge_requests/10289

MR

https://git.drupalcode.org/project/drupal/-/merge_requests/10289/diffs

Thanks for fixing those, seems we are getting test-failures now, re-ran twice

Okay, thanks for your time. I'll fix the tests. To be clear, are there specific tests that need to be passed or all of them?
I am not sure if some tests may be failing due to something unrelated.

patch for 11.2.2 without tests.

@sstapleton In general, tests are always expected to pass on all merge requests, any test failures are almost certainly due to the changes in the current merge request, as changes are not allowed in when they are causing tests failures.

@hmdnawaz Please update the merge request if there are changes you want to make.

Updated the MR for the latest 11.x. Unfortunately, the test is now failing. For some reason, the Revisions tab is no longer active (even though the code seems to be running fine).

Patch from the MR for 11.3.3

I created a new MR against main, with the changes against 11.x, because targeting main didn't seem to go over well. I took the easy way out by squashing the whole thing into a single commit.