Preview on forward node revisions throws an error

I can reproduce this issue. Updating the description with steps to reproduce. Also setting to Major.

I've confirmed this is also an issue with the latest code, 8.x-1.x branch.

After investigation, it appears the error is due to core not handling previews for forward revisions. Updating the description to clarify what is happening and moving to core.

I have added a recommended solution to remove the generation of HTML meta information on the preview page. It is a straightforward solution with low impact: the HTML meta information is likely superfluous given the previewed content does not yet exist.

Updating title to be more relevant. And pointing to relevant code:

Here is the method that is setting the "node_revision" URL parameter to null: Drupal\Core\Entity\Entity::urlRouteParameters($rel). The relevant lines are:

314     if ($rel === 'revision') {
315       $uri_route_parameters[$this->getEntityTypeId() . '_revision'] = $this->getRevisionId();
316     }

Since the entity doesn't yet exist, $this->getRevisionId() returns null, and since the "entity.node.revision" route does not allow the "node_revision" path segment to be null, the route generation fails.

Fix attached

Another WB bug moved to core #2708735: Creating a draft removes published node from taxonomy view.

I am looking at this issue to replicate.

I am unable to reproduce this using 8.2x drupal and 8.x-1.0+2-dev version of workbench moderation. Very possible I'm missing something...?

I have this problem on Drupal 8.1.1 and Workbench Moderation 8.x-1.0 and the patch in #6 seems to fix it.

Discussed this issue with @dawehner and we agreed that outputting link templates when previewing a node doesn't make any sense, so we should simply remove that code.

Also added a test for this. Since we can not get in this situation through the current core UI, the test just uses API calls.

+++ b/core/modules/node/src/Tests/PagePreviewTest.php
@@ -296,6 +296,18 @@ function testPagePreviewWithRevisions() {
+    // Check that previewing a forward revision of a node works. This can not be
+    // accomplished through the UI so we have to use API calls.
+    $this->drupalPostForm(NULL, [], t('Save'));
+    $node = $this->drupalGetNodeByTitle($edit[$title_key]);
+    $node->setNewRevision(TRUE);
+    $node->isDefaultRevision(FALSE);
+
+    /** @var \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver */
+    $controller_resolver = \Drupal::service('controller_resolver');
+    $node_preview_controller = $controller_resolver->getControllerFromDefinition('\Drupal\node\Controller\NodePreviewController::view');
+    $node_preview_controller($node, 'default');

Can you please create a dedicated kernel test? We want to get rid of executing php code in our tests.

@dawehner, the reason I kept the test there even if it's doing only API calls is that at some point we'll be able to test this in the UI, for example when #2725533: Add experimental content_moderation module lands.

How about I add a @todo to change the code to use the UI when we're able to create forward revisions in core?

+++ b/core/modules/node/src/Tests/PagePreviewTest.php
@@ -296,6 +296,18 @@ function testPagePreviewWithRevisions() {
+    /** @var \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver */
+    $controller_resolver = \Drupal::service('controller_resolver');
+    $node_preview_controller = $controller_resolver->getControllerFromDefinition('\Drupal\node\Controller\NodePreviewController::view');
+    $node_preview_controller($node, 'default');

This otherwise trows an exception. Let's use a try catch + $this->fail()

Improved the comments in the test a bit and added the @todo.

This otherwise trows an exception. Let's use a try catch + $this->fail()

Discussed this on IRC but I'll post here for everyone else: the try / catch pattern is for cases when we expect to get an exception, which is not the case here.

You are right. Thank you @amateescu

Committed cd87bb1 and pushed to 8.1.x and 8.2.x. Thanks!

Yay, thanks for this. Closing #2751501: NodePreviewController should not render entity links in the HTML head as a duplicate.