Preview feature broken if requested twice

i cannot reproduce this on drupal-8.0.0-beta12

Can you adjust one of the existing tests to trigger this? Not clear to me how the other issue could have broken this, I tried to be very, very careful about keeping BC.

This change should go in PagePreviewTest if anyone is trying.

That error is in comment.module. So I think it won't happen unless you have comment enabled and a comment field on that node type.

This has to be the check in CommentDefaultFormatter::viewElements(). Somehow in_preview is then empty when it shouldn't be.

I'm guessing this doesn't work anymore the way we it's supposed to:

unset($this->entity->in_preview);

in NodeForm::form().

Try to explicitly change it to set the value to NULL and see if that makes a difference.

Also compare in_preview in the preview() method and the actual controller..

I can confirm this issue is now in beta-13
this is the error i'm getting.
Fatal error: Call to a member function getEntityTypeId() on a non-object in ..../Sites/dru8/core/modules/comment/src/CommentForm.php on line 78

so, i figured i could help narrow it down by uninstalling the comment module but i'm not able to uninstall it with a fresh install... maybe related

it says

Allows users to comment on and discuss published content.
The following reason prevents Comment from being uninstalled:
Fields type(s) in use

next, i did a minimal install so the comment module would not be installed and the same steps create a fatal error with this message

Fatal error: Call to a member function getTranslation() on a non-object in ..../Sites/dru8/core/modules/node/src/NodeViewBuilder.php on line 98

this would suggest the comment module is not the culprit.

- nick

@BiigNiick See #2489472: Field-based module dependency uninstall message is unhelpful and not grammatically correct.

It means there are comment fields on some content types on your install. The message is not helpful, as you have seen.

Ah, thank you @cilefen.

so the error message is in an if statemement that shouldn't even be run

    if (!$is_in_preview)

the problem is probably in the section above where the $is_in_preview is set

  public static function renderLinks($node_entity_id, $view_mode, $langcode, $is_in_preview) {
    $links = array(
      '#theme' => 'links__node',
      '#pre_render' => array('drupal_pre_render_links'),
      '#attributes' => array('class' => array('links', 'inline')),
    );

also, it works as expected if you use the browser back button. if you click the 'back to content editing' button something is not set right, i would think

on line 42 this must not be setting correct?

            !empty($entity->in_preview),

- nick

it works if you comment this line out.

i know this is not a correct fix, but gets us closer to what's going wrong...

- nick

fail/pass

Looks good :)

RTBC + 1

+++ b/core/modules/node/src/NodeForm.php
@@ -88,7 +88,7 @@ public function form(array $form, FormStateInterface $form_state) {
-      unset($this->entity->in_preview);
+//      unset($this->entity->in_preview);

+++ b/core/modules/node/src/NodeForm.php
@@ -88,7 +88,7 @@ public function form(array $form, FormStateInterface $form_state) {
-      unset($this->entity->in_preview);
+      $this->entity->in_preview = NULL;

YAY! this works for me. I'm glad i could help find where the problem was ;)

the patch from #23 works as expected for me. +1 RTBC and ready for beta evaluation

It's a major bug so no need for beta eval we can always fix bugs.

Ha, this is timely. :) Just hit this bug a few mins ago while doing a live demo of D8. Fix looks great, as does the test.

Committed and pushed to 8.0.x. Thanks!

Assuming this is all good now, but noticed this error in 8.0.0-beta14; base install (no additional config or contrib modules), adding the first Article node for the site.