Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
node_is_page() currently doesn't work on revisions and node previews. Revisions and previews are per default displayed in the default theme. So certain preprocessing which is dependent on node_is_page() simply doesn't fire when looking at revisions or previews, for example the following lines are firing a certain CSS grid functionality on node/2 BUT NOT on node/2/revisions/1299/view AND NOT on node/preview/a67cfe71-e7fb-470f-be3a-42e2eac1846a/full:
function MYTHEME_preprocess_node(&$variables) {
$node = $variables['elements']['#node'];
$variables['neat_grid'] = node_is_page($node) ? TRUE : FALSE;
}
Proposed resolution
Change code of node_is_page() declaration in core/modules/node/node.module.
BEFORE
function node_is_page(NodeInterface $node) {
$route_match = \Drupal::routeMatch();
if ($route_match->getRouteName() == 'entity.node.canonical') {
$page_node = $route_match->getParameter('node');
}
return (!empty($page_node) ? $page_node->id() == $node->id() : FALSE);
}
AFTER
function node_is_page(NodeInterface $node) {
$route_match = \Drupal::routeMatch();
$route_name = $route_match->getRouteName();
if ($route_name == 'entity.node.canonical') {
$page_node = $route_match->getParameter('node');
}
elseif ($route_name == 'entity.node.revision') {
$revision_id = $route_match->getParameter('node_revision');
$page_node = node_revision_load($revision_id);
}
elseif ($route_name == 'entity.node.preview') {
$page_node = $route_match->getParameter('node_preview');
}
return (!empty($page_node) ? $page_node->id() == $node->id() : FALSE);
}
Remaining tasks
* reviews needed
* tests to be written or run
* documentation to be written
User interface changes
- None -
API changes
Unsure about that ...
Data model changes
- None -
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | Interdiff.3005029.5-10.txt | 7.73 KB | mikelutz |
| #10 | 3005029-10.drupal.Make-nodeispage-work-on-revisions-and-previews-as-well.patch | 7.57 KB | mikelutz |
| #5 | node_is_page_for_revisions_and_previews-3005029-5.patch | 1.14 KB | leymannx |
Comments
Comment #2
leymannxComment #3
leymannxFix code highlighting.
Comment #4
leymannxSame applies for node previews. So I updated the issue title, description and updated the patch to include previews as well.
The patch also updates wrong code doc where it says it would return an ID of the node, which never happened/happens under any condition in this function.
Comment #5
leymannxPrettified code.
Comment #6
leymannxComment #7
leymannxComment #8
tedbowWe can't change the return type as contrib/custom code could be relying on this return the ID.
Or actually I guess the return comment is currently wrong.
So I guess that is fine.
This seems risky to add.
The docs currently say "the full page view". So there is likely code out there relying on this returning FALSE if this is the preview or revision page.
So I see that it would be useful but I think it would break functionality on probably a good number of sites.
Comment #9
mikelutzLet me start by saying, I support having the ability to render revisions and previews in the same frame and way that node pages are. I've had to jump through hoops, and make compromises with clients in the past who have complained that 'things are missing' on revision review pages, and the problem is deeper than just node_is_page, it also involves block filters on content types and a whole bunch of other things that come together to render revision views differently from page views for nodes.
That being said, we cannot change the functionality of node_is_page(), because it would constitute a backwards compatibility break. Your use case is well served by this update, but there is going to be other code in contrib that calls this function, expecting it to only return true on node canonical pages, and we can't break their code. You can look into the core code to see the first example as to how this would/could break things.
It seems to be used only 3 times in core code.
Once by the book module to alter the links for the book,
Once by the statistics module to add the statistics to the node render array on the page,
and the big one: in the main node preprocess method, it's used to set the node's 'page' variable to true, which is what a lot of themes and process function and preprocess functions depend on to detect if we are in the node page view. The in-code documentation there says
(As an aside, we already handle preview mode there. Again, I believe revision pages should have this set, and I often preprocess nodes in my own theme to add the 'page' variable on revision pages to make them appear more like the main node page for my editors.)
Your patch changes the occasions when this variable is set, but doesn't update the documentation here. Even if it did, it's again a backwards incompatible change, as themes in core and contrib expect this variable to be set as currently documented.
From a back-end code perspective, the correct way to do this would be to create a new is_node_page_or_revision_page() method with your logic, and possibly deprecate the is_node_page method in Drupal 8 and require everyone to switch to the new function by Drupal 9. I don't think there would be much support for adding new procedural code in core at this point though. If anything, this feels like it should be abstracted for all entities and generalized into a service somewhere, (see taxonomy_term_is_page()).. It doesn't quite feel right for EntityRepository, but that's the closest I can think of, and I don't think it warrants a new service on it's own, but you could at least have an api to detect if you are on an entity page, and optionally include canonical and revision routes or something like that.
This would provide an api for themes to start using, but it's still going to be an uphill battle to get them to switch over.
Comment #10
mikelutzThrowing something against the wall here. This moves the procedural functions node_is_page() and taxonomy_term_is_page() into PathMatcherInterface::isEntityPage(). The new method takes an optional array of link templates to compare to, so you can use it to determine if you are on a canonical, revision, preview, or whatever page, and abstracts to entities.
If we like the approach, we need tests.