Node previews are cached by dynamic page cache

This patch applies cleanly to Drupal 9 also

this patch is correct and definately works

Reviewed and tested by the community:

the patch is tested in Drupal 9.0.9. It works perfectly.

Thanks @fago for providing the patch.

the patch is tested in Drupal 9.0.9. It also works perfectly for the anonymous preview node behavior

Thanks @fago for providing the patch.

Great fix, hopefully an easy adjustment to the tests can be made to cover this.

Yes this needs some test coverage, but the change makes sense.

Added test coverage.

Thank you so much for the tests!

Patch Has tests,
RTBC, verified this ourselves, solution is working perfectly, cache is expiring as expected, automatically handled now thanks to this patch.

Was very tricky to debug this and find that we needed this fix, so I highly recommend the fix.

It should be a safe fix for 9.0, 9.1.x , 9.2.x, we've tested it extensively on 9.0.x and 8.8.x

oh one last thing, someone should upload the tests-only patch to make sure it's failing.

Great job @anmolgoyal74 and @fago for the original patch and JamesYao also for his great detective work on this.

The test alone does not fail for me.

hmm,

new patch, tests checks both title and body field.

The test alone does not fail for me as well.

The trick to the test is we need to edit , preview, edit AGAIN, then preview, IRL without the patch the second preview shows first preview values

needs work on tests.

I am facing this issue with an anonymous user only. And with node_test enabled, the test only patch does not fail.

Nice work anmolgoyal74 , yes that is our use case also.

tests only patch works, just hiding it so that it doesn't re-trigger automatically.

+++ b/core/modules/node/tests/src/Functional/PagePreviewTest.php
@@ -500,4 +501,28 @@ public function testSimultaneousPreview() {
 
+  /**
+   * Check that previewing a node after it has been previewed and not saved
+   * with dynamic_page_cache module enabled doesn't cache the previous changes.
+   */

This should just be one line, something like 'Tests node preview with dynamic_page_cache and anonymous users'.

Updated the patch in #24 with the changes suggested in #29. Thank you!

suggestions in #29 addressed in #30.
I think it is good now for RTBC.

Committed and pushed ab9919c356 to 9.2.x and bee62f645e to 9.1.x. Thanks!

Backported to 9.1.x as a container rebuild will happen on the next 9.1.x release and that'll allow this to be fixed with no update function.

Thanks, nice one-liner!

My browser