Problem/Motivation

The node variable is provided by template_preprocess_page, but it's only available for the canonical page. This is prone to bugs. For example, in Umami the page title is displayed twice in the node preview page because of this.

Proposed resolution

Make node variable available in page.html.twig on the preview page.

Remaining tasks

User interface changes

Before

before

After

after

API changes

node variable is not available in page.html.twig on the preview pages like it is on canonical pages.

Data model changes

Release notes snippet

Comments

lauriii created an issue. See original summary.

lauriii’s picture

Status: Active » Needs review
Issue tags: +TX (Themer Experience)
Related issues: +#2736391: Node type body class is not present on node preview page
StatusFileSize
new1.33 KB
lauriii’s picture

Issue tags: +Field UX

Tagging this with Field UX even though this is not directly related to Field UI because this is still related to the user flow of configuring a content type.

smustgrave’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative
StatusFileSize
new1.64 MB
new1.99 MB

Confirmed the issue on Umani demo.
Patch fixes the issue

Added screenshots to issue summary to show before/after

andy-blum’s picture

+1 RTBC

Discussed with @laurii in slack the idea of adding a check of \Drupal::routeMatch()->getParameter('node_revision') to ensure revision pages also get the node object, but it seems the node_revision parameter is always accompanied by the node parameter. Ultimately decided checks for node and node_preview cover all use cases.

catch’s picture

I think we should be trying to deprecate this rather than using it more widely. Why is it happening in template_preprocess_page() and not node_preprocess_page(), and/or why isn't it generic for entity types - if it's really needed at all. Having said that, maybe we want to commit this and look at it more widely in a different issue.

lauriii’s picture

+1 for looking into deprecating the node variable in a follow-up. FWIW, this is not adding additional usages, it just fixes the current, somewhat broken behavior. This also addresses usability impacting bug in Umami.

bnjmnm’s picture

I think removing node could be disruptive to many sites, even if core doesn't use it heavily. However, I think a followup to discuss that should be created, and once that exists I'd be inclined to commit this 🙂

lauriii’s picture

  • bnjmnm committed 36145a51 on 10.1.x
    Issue #3342891 by lauriii, smustgrave, andy-blum, catch: 'node' variable...

  • bnjmnm committed cad17767 on 10.0.x
    Issue #3342891 by lauriii, smustgrave, andy-blum, catch: 'node' variable...
bnjmnm’s picture

Version: 10.1.x-dev » 9.5.x-dev

Committed to 10.1.x, cherry-picked to 10.0.x. Will backport this non-disruptive fix to 9.5.x after confirming tests pass on that version.

bnjmnm’s picture

Status: Reviewed & tested by the community » Fixed

Cherry picked to 9.5.x as well.

luenemann’s picture

Status: Fixed » Reviewed & tested by the community

  • bnjmnm committed ead0950d on 9.5.x
    Issue #3342891 by lauriii, smustgrave, andy-blum, catch: 'node' variable...
bnjmnm’s picture

Status: Reviewed & tested by the community » Fixed

Thanks @luenemann, looks like the push didn't take. This is up now.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone’s picture

Published change record.