Allow to preview content in an actual live environment

Source file, ideally we tweak the proportions a bit in an actual working patch.

Implementing this properly will require an upgrade to how we store temporary data.

Right now, in Drupal, we store temporary data in a cache by caching the generated form. In order to preview, we have to process that form and reassemble data. This is fine when you click preview, but in a system like this, you're actually visiting a different URL from the form.

Logically, we could put the form build id in the URL and load the form and process it, but there are several weaknesses with that:

1) It is wasteful (processing a form that was not submitted)
2) Form build IDs are sha generated strings. There is no way to reproduce it if you don't have it. This means you can't do content locking.
3) It continues the trend on Drupal of the form owning the data when the form typically should only be a representation of the data.

If you're familiar with Views, you've probably seen that once you start editing a view, the view is 'locked' to prevent cross editing from another user. This is something we absolutely should have with content as well. Right now, the only protection we have is that if someone else actually submits an edit before you, and the timestamp changes, your submission will be rejected. This is, however, *after* you've made all your edits and your edits are rejected, whereas the Views method happens *before* you've made your edits so you do not waste your time.

This is accomplished by using a separate non-volatile cache (and thus it's not truly a cache) for temporary storage which is tied to the session ID. (Note: Tying to the session ID is something I would loosen in core). When editing begins, this record is created; the existence of that record for a given view is considered a lock. A mechanism is provided to break stale locks, and locks > 14 days old are automatically wiped by cron.

The actual implementation of the underlying layer is very simple: http://drupalcode.org/project/ctools.git/blob/refs/heads/7.x-1.x:/includ...

I don't use a simple key-value store (i.e, CMI) for this because that would make it much more difficult to do the locking aspect. I personally think the locking aspect is very important. Joomla! has had content edit locks since the day I first looked at it, back in 2006. They were clumsy at the time, but even then I thought that such things were essential, which is why Views have them today and I have never once regretted that decision.

IMO, to implement the above preview, we should implement something like my object cache system in core and implement a layer directly with entities so that all entities, everywhere, can automatically have content edit locking with no more difficulty to implement than editing the entity.

I personally would prefer the CTools approach because it would also solve the "Oops I left the page" problem. I'll never forget the time I was teaching a class with 8 computers plugged into a powerstrip that blew right in the middle of everyone configuring views. Everyone was immensely impressed that Views could pick right back up where they left off after the whole class just had their computers shut down on them. Using the temporary object store approach would also give us the locking and auto-saving possibilities that Views has had for years. Not to mention contrib could use it too.

Forward-saving revisions could also solve the preview problem, though the niceties and reusability of the object store make me lean towards that approach. From a usability perspective, I don't think "Preview" and "Revisions" is something that people will immediately be able to make a connection between. I wouldn't expect a preview to create a revision, since I haven't saved anything yet.

I lile the idea of ctools in core, and I *love* the idea of fixing previews :) +1 for everyone working on this!!!

Side note: I would imagine that if improved previews were powered by revisions, that most of the revision aspects would actually be hidden to the end users by default (only if revisions are "actually" enabled for a content type). For example, the 'Save and preview' button would save a draft, unpublished revision. Editing that node would continue editing that draft revision or be silently creating new revisions on each edit. Clicking the 'Save and publish' button would publish the current revision being edited. Using revisions uniformly across multiple entity types requires a *lot* of cleanup to revisions in core (very hard-coded to nodes), but it may be more worth it in the end by improving revisions and better tying it together in the UI.

Let me also add:

1) A LOT of fixing of revisions would have to happen in order to have forward revisions working. For example, revisions are very node specific. They would have to be genericized. You can currently only save the current revision -- field API would have to be updated to let you save arbitrary revisions. (Note: Both of these things should happen regardless.)
2) This would leave out any entity that does not support revisions (either because they do not want or do not need them).
3) I doubt the ability to make revisions invisible.
4) Failed editing could lead to a proliferation of revisions -- typically in this kind of workflow (i.e, Wordpress which is where some of the inspiration is from) you'll "change your mind" about revisions and cancel. But in some proposed systems, revisions are never deleted. They'd just hang around. Need to be careful about that.
5) No matter how much you want to make the UX easy, I don't think it would be. Once you start using revisions this way, it becomes intrinsic to their nature when they are administered.

EDIT: My bad, 3 and 5 are basically the same. I've had a little to drink. Sorry. :)

Temporary data doesn't need an entity id. Typically I use a key that means "there is no id assigned to this data yet". In fact, this works better than revisions when there is no entity id; you cannot have a revision without an entity id, since the key is compound. The object cache has no such limitation.

Also, it's not a form cache. It's an object cache. You store a fully formed entity object in the cache. The form is merely what you use to transform it.

These options are definitely not mutually exclusive. From the arguments made above, a CTools-style object cache is the better candidate to improve previews and it's help with auto-saving would be a huge win too.

Improved revision handling should be done too but it does not need to impact core previewing.

So to move forward, where should CTools-cache-in-core start? There's a meta-issue tracking entity changes. #1346204: [meta] Drupal 8 Entity API improvements One of the main changes is converting entities to classed objects. Merlinofchaos, does that make caching any easier/harder/irrelevant? Or should this be done at a level deeper than entities so that the same system could cache entity objects or CMI objects?

Looks like a plan. Then point 1 is the next step for the code, point 2 is the next step for the design. I propose merlinofchaos & friends do the code discussion and ux team works on a more fleshed out design/prototype :)

Thanks.

I think both setups are extremely useful, and at the end of the day I think it'd be awesome to be able to put them together. Fixing revisions so you can use them for more than just history is valuable all on its own, and having temporary data so you can lock and better support multi-step content forms would be a bigger win now.

Yeah, this captures my feelings pretty well, too. I don't think this is an either/or situation. I think it's an "and" situation. "Future revisions" solves one ginormous class of problems, but in terms of both new entities, as well as the "Oops, my cat just stepped on the power strip" situation, the object cache seems like a good direction for solving the preview stuff.

Can we make sure there is (if there's not already) an issue about allowing forward revisions in core? Because that's also really important. #218755: Support revisions in different states is at 110 comments and has no issue summary so I can't quite tell if that's it or not.

Oops. Didn't mean to cross-post.

I have marked #1167242: Expected a realistic preview of changes. as a duplicate. Although it came first, this thread seems to have more direction.

@Bojhan. I like your mockup in #1, I'm not sure it needs the 'full node' select list, especially with the term 'node' in it. I also think it needs to be clearer that this is a preview. Imagine the use case that someone aims to click save, but clicks preview by mistake. They get this page which looks like their post is saved and looks good. So they close their browser and loose their work. For this use case I also think there should be a save button on the right of the preview bar.

Note that the following is totally out of scope for the initial patch, but.

We should probably anticipate if not core then at least contrib wanting to stick more options on the preview bar. For example, preview as a given role, preview in a given language, preview as a user from a given ip2geo location, etc. That one drop-down could quickly become 7-8 drop-downs, some checkboxes, a calendar widget, and a partridge in a pear tree. How do you anticipate the design dealing with that?

At least a "theme" selector would be needed for multi-themed sites

My 2 cents : Have we considered using an iFrame to preview content directly from the node form ?

Is CTools object cache flashed when all caches are cleared? Would this flush any auto-saved data too if they were based on that?

Looks like this needs a starter patch then!

...sorry.

It's not truly a cache. When I rewrite it for core inclusion I think I will call it a TempStore instead.

Which I believe we should also use instead of the for the extremely abused "cache_form" table, which isn't really a cache either and also shouldn't be flushed with cache clears.

Crickets ...

I've gotten a start on the TempStore object. I haven't created an additional issue (perhaps there should be one) and I haven't had a chance to finish it. There's a lot tugging on my time right now, but it's important to me for many reasons.

TempStore is in, this should use that!

Here's the change notice with some examples: http://drupal.org/node/1805940

Can you explain why this is more than a feature request?
Also, the issue summary is fairly out of date.

Usability problems can hardly be categorized as feature requests, sadly that has been the trend of the past few weeks. We slowly moved from bug -> task -> and now feature request is the new trend. However as far as I am concerned this is a serious issue that affects most people using this functionality, that we need to resolve before final release.

I can't update the issue summary, it seems like it needs to have a new technical approach.

Some considerations:

• Hook invocations. Since the entity isn't saved yet, hook_entity_insert() or hook_entity_update() hasn't fired. Which is by design I think, but might affect how actual the actual content preview is.
• Access. The aforementioned unfired hook invocations might change the entity's access restrictions. So we probably want to restrict access to the preview to the owner, or perhaps administer content for nodes.

First stab at this, needs a lot of work, but could use a lot of reviews/tips on how to implement this cleanly. Still lots of things to cover like images, tags (and more stuff along the road I guess).

Note that I won't dedicate full time on this, so feel free to hack away at this at all times, assign yourself or ping me on IRC in case so we don't do double work :)

Attached a screenshot of how it currently looks like - the link to go back is at the top left.

New patch: uses uuid, you can now switch view mode as well, css needs love :)

Great progress! Interdiffs would be useful though :)

Quick re-roll after #1838114: Change node form’s vertical tabs into details elements went in. Now *really* off.

The last submitted patch, 1510544-62.patch, failed testing.

This version fixes the newline in the css file and makes tags work again when reloading the content

// @todo can we do this somewhere different ?
$tempstore_id = drupal_container()->get('request')->query->get('tempstore_id');
if (!empty($tempstore_id) && ($data = node_tempstore_load($tempstore_id))) {
$node = $data;

+ // We have to prepare the node to fix the autocomplete tags widget.
+ // @todo now we do a double prepare... Is that allowed? Probably not...
+ node_invoke($node, 'prepare');
+ module_invoke_all('node_prepare', $node);
}

Second try....

Ok some more digging:

My patches above are incorrect they just seemed to work because I was dealing with existing terms.

Problem:
When defining new terms in an autocomplete, they are not yet saved, they get a tid "autocreate" when going to the preview.
When going back to the node form all the "autocreate" tags can't be loaded so they don't get rendered.

This feels like a general issue with references that don't exist. And probably needs a general solution.
I have no idea how the autocomplete system works but this seems to me the only situation where this could happen.
If this is a central system it would be cool if we could fix it there.

But I fear this needs a unique solution for each entity that is facing this problem.

This patch should turn back green, removed some todo's as they are actually ok, did some fixes for node preview titles etc. I think after this, it's time for interdiffs. I've asked a colleague frontender to look at the css and the javascript this weekend.

- edit - note that I don't necessarily consider the code for taxonomy dirty, it's just the way that its autcomplete feature works, however, optimization is always possible of course .. :)

Nice progress — the code already looks much better!

We can move the access check to the hook_menu (in stead of returning true). The doc has to be changes as we return a render array now. And not pure html.

I should have used $tid everywhere here

I considered this dirty because it is duplicate code. We can refactor this to be a bit cleaner.

We can probably just remove the tempstore_id property completely and use the uuid. So replace all tempstore_id everywhere with uuid.

return FALSE instead of 'return;'

- edit - we also need a test for switching the view mode on the preview page and maybe also a 'clickLink' on the button at the top.

Moshe also suggested to use a lock mechanism like views does, so we can finally change the mechanism where you'd potentially get a 'This content has been changed by another user' when trying to save a node. Basically create the tempstore object immediately after creating on node/add or node/x/edit and then checking the expire time we can add a message like 'This node is being edited etc'. This would solve the problem even for anonymous users.

- edit 2 - after actually checking the user.tempstore class the user with aspilicious this should actually be *really* easy and probably working fine for everyone :)

However, I'd leave that for a follow up, although I don't think it would be *that* much code.

@Bojhan - The font comes from Bartik because we are in that theme.
Also, do you have an idea how we would handle locking nodes ? The idea is that when a user tries to edit a node which is edited by someone else would get a warning that this one is locked and can potentially (with permission) can break that lock, so just like views. We can do that in a follow up if/when (crossing fingers) this goes in.

The locking of nodes isn't actually even in the OP, though it could be built on this.

This is just about a true preview, not just "your words maybe run through a text format".

Missing dot in the comment. Let's use #type 'value' instead of hidden.

Missing dot

complete ?

- note - we'll upload a new version with css and js changes as well

I would be concerned that actually disabling links will be a confusion for users. Links that are disabled need to be visually identified, can the whole page except preview and the button be greyed out for example? Or JS warning attached to clicking somewhere else?

I'm confused why add a new layer if the patch already have a way to prevent clicks to work.

@meba I dont know, this could be something we split of from this issue as a followup. As its ideally something we explore a few options on, for several reasons;

1. Some users might want to check links to see if they work
2. Not being able to click links is a work around for not having auto-save ability, which with temp store and other tricks we can now actually do
3. Preview is about being an actual preview, not a different one - if we make it visually different by disabling links it will create a different experience (possibly requiring explanation) - most likely even harder on custom themes - requiring "disabled" looks for all different kind of links.

However these are all somewhat assumptions, because in our current preview screen - we hardly see users click the links. Its just not a common use case, however this is a completely new model - so it might become a more prominent use case. With all this in mind, I would suggest a good follow up issue.

#91: Apologies — I didn't know that was already implemented. I was just providing a suggestions as per #90.

#93.1: we could make it so that only links inside the previewed node are clickable. Or potentially add a Google Docs-like tooltip when clicking the link.
#93.2: It doesn't make any sense to click links other than those inside the previewed node's contents when previewing a node. If you disagree, please explain why. So: I disagree it's a work-around for not having an autosaving capability, IMVHO it's common sense.
#93.3: I wasn't arguing for disabling links in the sense that they would no longer be <a> tags, or would get some class attribute. Nothing should look differently. But a user should not be able to start navigating elsewhere, and if he should, then he should explicitly confirm that first.
#93.conclusion: I agree, this the perfect example of a follow-up-able issue.

#94: It's not very clear to me what you mean, but I presume it's a reply to #93.2? Because of the temp store, they wouldn't lose everything, but you'd lose your context, so in that sense I agree :)

Just open a parenthesis on the feature raised by @merlinofchaos about content locking in #3. I use Drupal 6 on an intranet with 1800 users based on Organic Groups and it would be impossible to implement it without content locking.

You probably already know, but worth mentioning, the Content Locking module do it, and do it very well. It is absolutely perfect for our needs and it has the entire workflow cited by @merlinofchaos. It's a great reference.

Autosave is also very welcome and necessary, but it is essential to lock content in a collaborative environment.

(Sorry for my english)

Looks good to me, I tried an earlier patch. There is probably some follow up work, e.g. making the select list nicer, disabled links. But from my point of view this is a really good initial go, that seems to achieve all that we set out to do.

Lots of feedback. Boatloads of nitpicks, but also a few big questions, amongst one which is a very fundamental concern — also unanswered by the rest of this issue AFAICT.

Good catch :)

All this metadata should be stripped from the PNG file, but this can be a follow-up commit. UPDATE: considering the loads of other feedback, you may want to fix this now. Optimize it using http://imageoptim.com/ (i.e. which uses PNGOUT, AdvPNG, Pngcrush, extended OptiPNG).

Does this mean we only support node entities? Is there a reason we can't do this for all entities generically?

Or is that a follow-up issue?

I saw some discussion about this in comments #9 and #11, but it's not explained in the issue summary. So can we please clarify this?

Is it still allowed to manipulate $_GET directly? Shouldn't we be using drupal_container()->get('request') to modify this?

the -> a ?

s/id/ID/, no?

Leading slash missing.

Extraneous newline.

Should we still use menu_get_object()?

Another extraneous newline.

Missing leading backslash.

array $form

array $form, array $form_state

"add default"? Sounds strange.

I understand why you call this variable "link", but wouldn't "path" be more in line with the rest of Drupal?

Maybe $current_view_mode instead? That makes the default value setting for the select less weird, twenty lines further.

Should be removed.

s/node preview select form/node preview view mode select(ion) form/

array $form, array &$form_state

"@return bool" ?

s/css/CSS/

s/user/author/ ?

This comment is unnecessary, especially because it's already explained in a comment a few lines higher.

Bad sentence, and it doesn't seem to match what the code is doing?

Extraneous newline.

Why are changes to this function necessary?

s/complete/completely/

Additionally, we need to check the accessibility (and usability, for that matter) of just having a dropdown in the middle of a blue bar that says "Full". Methinks that should have a label of some kind. The feature in general needs to be tested as well to ensure it passes the accessibility gate before it can be committed. Hopefully since this is just largely re-using stuff already in Views there won't be a problem here.

This doesn't need to be a commit blocker, but one thing we should think about is making this preview bar something other modules can easily hook into, so say for example the http://drupal.org/project/ad_geoip module could preview how the node will look if the user comes from Spain vs. Canada, and http://drupal.org/project/browscap could let you preview it on such and such browser, and so on. Right now, implementation-wise, this is highly tied to nodes, and node is an optional module. It would be cool if node was simply implementing hook_preview_bar_alter() (or whatever) instead.

The last submitted patch, node_preview-104.patch, failed testing.

The last submitted patch, node_preview-106.patch, failed testing.

Novice task for this issue: test the patch against the core accessibility gate. Four parts of the accessibility gate are straightforward to test:

1. HTML structure meets WCAG 2.0
2. Text color has sufficient contrast
3. Form fields are labeled
4. Javascript is keyboard-usable

Choose one (or more) of these points, apply the patch and clear your caches, and follow the instructions provided for each in the accessibility gate information. Post a comment describing your testing and the results here on the issue. Thanks in advance!

@xjm - There's no label on the view_mode form to switch between full & teaser modes.

Took a brief look at the color contrast. With white text & a gradient between #4F9FEA & #4481DC for the background, unfortunately it fails for all but large text (and this isn't large text) - http://webaim.org/resources/contrastchecker/

Other than that it looks fine. Keyboard navigation was fine & http://wave.webaim.org/toolbar didn't complain about anything else.

Will be a good addition when we get those two things cleaned up.

@mgifford Is that white on blue related to this patch, seems like an overall issue with that button then.

The last submitted patch, node-preview-1510544.114.patch, failed testing.

If I enable comments on the node and go to preview, I see no comment form. If I then go back to editing the comments are back disabled.
Also menu options (and it seems other options).

I am also not sure why I see strange flickering once I click on Preview - the site first opens the page in Overlay and then switches off the overlay.

The preview does not belong in the overlay in my opinion, the intent is to use the actual theme, not the admin theme.

True, I guess this is the same behavior of the block demonstration page.

I don't think we'll able to get the menu's working easily (although theoretically possible I guess), comments will be a bit easier, however, dataloss when going back is something we'll have to look at.

Swentel showed me this today. It already works so naturally, it's basically not exciting anymore! Which is a good thing, because that means it does what is expected.

I think the view mode label should be visible.

Awesome work.
I just have one suggestion. Can we have save button next to 'Back to content editing' link? So that if I have done editing I can save content immediately. Maybe we can do it in followup if it is out of scope.

Lets do that as a followup, its a little bit of a can of worms - because you might want to include revision data as part of this - what is "saving"? etc.

For the label, how about just "Mode:" or "View mode:" — doesn't seem too technical for a content editor, who should at least understand there are different ways a bit of content will be displayed.

Added #1899360: Add save dropbutton next to 'Back to content editing' link in node preview. follow-up as per #123, #124, #125 and #126.

Hah. Thanks for looping me in. ;)

Interesting issue. To my knowledge, one of the top-notch Drupal agencies scoped a feature like this to require a mere ~$500,000 budget for being developed. Of course, that included to take care of the utter pitfalls and consequences a functionality like this would entail.

The changes to Menu module in this patch pretty much demonstrate the problem space: As long as you assume that you're on your own and on your own isle, you can do whatever you want. But as soon as you take multidimensional modularity into account, any kind of simple idea blows up completely and leaves the overall system in a state we definitely do not want to be in. In turn, that gets you pretty quickly to the aforementioned budget.

This patch does not address that. It ignores the problem space of entities getting enhanced by modules, which, each, have their own storage, and which need to participate in the rendering process, but whereas the rendering process relies on the previously mentioned storage. Worse, the submitted, entity-specific form values can have a significant effect on other parts of the page/site, outside of the rendered entity.

Due to that, it is only a showcase of a hypothetical feature. The feature cannot be implemented in the proposed way, as it would have massive implications on contributed modules down the line.

However, I do think that it would be very beneficial and worthwhile to actually leverage the most recent improvements to the entity system, in order to make a relatively big leap in the right direction for Drupal 8. Details are following below.

So, alas, most of this is not the front-end related review that has been asked for, but instead, a much more significant, architectural review:

1) I do not understand what this "trick" is doing, and, which consequences of this manipulation have been considered (and thus are expected) and which have not. Right now, I assume that plenty of consequences were not considered, since this entire feature request is a million-dollar effort in the current architecture, and when accounting for all consequences.

2) I don't understand why the conditional code futzes with the link's attributes.

3) Why is the preview shown in a form in the first place...?

Arrow buttons are a thing of the pre-HTML5 era.

A preview like this, on a separate page, should not call into the entity form controller in the first place. That's view vs. edit.

OK, so here's the utterly funny part about this:

Ever since, I'd argued and preached that we simply need to save content entities, if we want to preview them. Or in more common speak: Autosave.

You're essentially doing exactly that here — merely with the exceptionally complex workaround of going through a custom tempstore to achieve a preview.

The tempstore approach prevents each and everything else from working like it is supposed to work — which essentially circles back into the Menu module change: Any module that integrates with the content entity suddenly has to take extra care for the possibility of a fake-stored entity in a tempstore, and adjust ALL of its code to work with that edge-case.

So here's the essential question:

Why do we even try to store something in a fake storage and invent tons of custom plumbing for dealing with that, instead of actually storing it?

Make "live" previews limited to entities that can be stored and have a publishing status. — No one will complain.

We've significantly improved entity revisions for D8. — The moment you click Preview, you surely have reached a state of art that is worth to backup in a new revision.

If necessary, we can look into "tagging" auto-created revisions later on, so as to prune and clean them up upon final save.

However, the big, important, and essential difference is this:

1) We're a Content Management System. We care for your content, even if you merely hit "Preview".

2) We do not force 13,000+ contributed modules to deal with a (strange) concept of unsaved-but-then-again-saved-but-then-again-not-for-realz entities that may or may not be rendered.

3) Instead, we pave the way to actual, site-wide previews, which actually include entire site sections, content, menu link tree structures, permissions, and whatever else is needed. This is the multi-million-dollar part that will take until D10 to be figured out in total. But at the very least, all modules are able to rely on clearly defined concepts (revisions) in working their way towards the New World Order™.

And of course, everything else related to this, is mostly baked-in already: Permissions to view a (unpublished?) node in preview? A router path to view a node revision? Differences to your last preview? etc.pp.

Can we reverse this into /node/%node_tempstore/preview ?

It looks like this form should be in node.pages.inc.

Or even better, part of NodeRenderController. That is, because this form is attached to a rendered node entity.

That said, the view mode to render the entity in should be part of the router path; i.e., /node/%node_tempstore/preview/%[view_mode]

Or even more appropriate, leave it out of the path, but turn it into a ?view_mode=xyz query string parameter instead.

In turn, this form can be changed to use 'method' => GET, which in turn eliminates the submit handler, and instead, moves the view mode processing into the actual code path that should care for it.

Minor, in terms of the overall patch: ;)

As you already figured, any kind of gradient or special background, positioning, and any other fancy visual styling doesn't belong into module CSS. :)

The last submitted patch, node-preview-1510544.132.patch, failed testing.

I don't think that's possible, since #ajax and a couple of other things rely on the form cache.

That said, this code pretty much resembles what Form API's form cache is doing already... unless I'm terribly mistaken, the entity is cached in there already.

It looks like you should be able to simply leverage the Form API cache by doing this:

node/%nid/preview/%form_build_id

And with that:

$form_state['input']['form_id'] = 'node_form';
$form_state['input']['form_build_id'] = $form_build_id;
$form = drupal_build_form($form_id, $form_state);

(give or take some details)

That said, I still don't understand why this code gets executed in the code path of the entity form.

@meba Where do you get that number from - with our long tail I imagine its far lower? I am skeptical about auto-save, not because it wouldn't be good - but because all these "modules" that will then integrate, to me sounds like a dream. I have been waiting for years to get any type of preview on blocks/menu's etc, and no one is interested. Unless anyone offers to help out on getting those parts integrated with this, I dont see much point to swentel turning it all around - also lets keep in mind what Edit module does, that uses tempstore almost entirely?

@aspilicious: There have been significant improvements to node/entity revisions in D8 (#218755: Support revisions in different states) and we continue to work very hard on that (#1643354: [Policy, No patch] Overhaul the entity revision terminology), so what you're saying was the behavior of D7, but with all the entity revision improvements, we should be in a state that allows for that. Unless I'm mistaken, that's what seems to be called "forward-revision" (#1776796: Provide a better UX for creating, editing & managing draft revisions.).

@moshe:

2) There's admittedly some confusion around the term "autosave", potentially caused by the contributed Autosave module. However, all client-side editor library plugins called "autosave" that I know of are actually issuing a HTTP request to the server, in order to simply save the content being edited by the user on the server-side, because the client-side is not a reliable storage and not what a user expects from a system that claims to manage content (e.g., switching between clients, browsers, power outages for more than 6 hours, whatever). Which in turn brings me to:

1) I think I've sufficiently clarified that unpublished draft revisions will not magically work. In fact, I'm relatively sure that entity-integrating modules are full of bugs right now (e.g., Menu module creating a publicly visible menu link for a unpublished node/entity). However, what I'm saying is this: We need to cater for that anyway, because draft revisions are absolutely going to happen. So why do we introduce yet another possible entity state + storage, next to draft revisions, which adds a considerable amount of complexity across the entire system?

In the end, the experience that users expect from a system that claims to manage content is exactly this:

Just save what can be saved, and manage the content for me. Don't make me think of "save." And, screw you if you ever fail to save my previous work.

That's the today's experience of Google Docs/Drive, and alas, also WordPress (apparently, since years already, since they're using the TinyMCE Autosave plugin, which essentially triggers a form submit every 30 seconds or so). What both have in common is that they truly manage the content for you and make you forget about "saving".

Additionally, both are automatically aggregating, pruning, and cleaning up needless intermediate automatically saved revisions in between (hence, "autosave"). AFAIK, WordPress achieves that by "tagging" auto-saved revisions, in order to identify them later (but since it's just WordPress, they're [ab]using the revision log message field to contain an auto-generated log message, and simply querying on that unique string identifier in the revision clean-up operation).

In the end, I'd like us to move on (and actually forward) on these pathways:

1) Become an actual Content Management System. It's surprising that we're even listed as one currently. Drupal is a pure Input/Output layer, there's factually no user content that is actually "managed". Entity revisions are the key facility towards that.

2) Leverage the architectural functionality that we already have, instead of inventing a new, custom "temporary storage" for user content.

3) Push the entity revision efforts forward.

4) Pave the way to CRAP (Create, Read, Archive, Prune) instead of CRUD (..., Update, Delete).

5) Pave the way to previews of complete site sections, instead of just a single entity.

In essence, I'm not saying that the tempstore approach cannot work out. But what I'm saying is that it's step in a very wrong direction. Technically, architecturally, and also from a product perspective. The web and end-user-facing technologies and applications are on their way to completely abandon and leaving behind the stone-age approach of a form that needs to be submitted or saved. Instead, users expect to interact with something, and as soon as they do, the app takes care of making sure that the user's interactions are persistently recorded somewhere. That is what enables "Undo" and "Redo". That's what enables you to forget about "Save". That is what gets you draft revisions that you can compare and revert, *before* making your post final.

In turn, a "Preview" button that saves into a tempstore, which will inherently forget about my entire precious work in X amount of minutes is the exact opposite of that. All we really gain from that is a fake-preview of content, which is inherently limited to exactly the entity itself, by design. It won't be able to support anything beyond that in the future. And it's totally not what end-users expect from a CMS in 2013.

@sun Could you perhaps shoot a helping hand on this one, it looks like we are fully willing to do it - but it requires some hardcore knowledge of Form API. I agree that in the end we want entity revisions and with that preview, on everything - so this is the road to go, but I think swentel is struggling to make this happen given the timelines.

I am actually going to move this to bug report, I have seen users get confused about this on many occasions throughout usability tests. This is not a "nice-to-have", its an essential part of a CMS that Drupal does not support. Being able to preview content before, one presses save is an expectation many users have and the fact that we offer it with a incredibly confusing experience should warrant it as a bug (it's not really a new feature, as much as fixing a incredibly broken one).

Let's not fool ourselves here. We are *not* going to tackle preview in a live environment in Drupal 8. I'm moving this as a feature request to Drupal 9.

The main problem of this kind of feature, other then the reservations that @sun raised, is that a piece of content never, ever, works in isolation. Pieces of content are displayed in Views, put in Menus, pulled by Panels, displayed in Blocks, sliced in meta-tags, indexed in Solr, etc.

This patch hardcodes the preview as the full page view, this is not going to be useful to anything except trivial sites.

The problem space described here is the one of the "Future preview", which is one very complex beast to get right. We have built full-site previews before, but that was by content staging / synchronization between Drupal instances. Building a real preview mode inside a single instance of Drupal requires all the components to be aware of the preview (including Solr indexes, database denormalization etc.), so it needs to be deeply rooted at the core of the system.

Adding some half-backed preview right at the last minute will not do Drupal 8 any good.

Given that we have the option to edit in place now, which is the best preview you can build in a non-hardcoded way, why not simply removing the preview option from the node module?

I'm joining this issue late, but I quickly skimmed through the recent comments. I think I agree with sun that ultimately, we'll want preview functionality to be based on unpublished forward revisions, but until the work in #1776796: Provide a better UX for creating, editing & managing draft revisions. is settled, I don't see a problem in proceeding with the tempstore approach. Tempstore is now as much a part of core architecture as $form_state is, and better suited to preview functionality, because $form_state is limited to when you stay in form context. In fact, maybe when we want to refactor previews to pull from an unpublished revision, we'll actually want to make that swappable/configurable, so that whether it's tempstore or a revision can vary.

I also agree that this issue is a bug fix more so than a new feature. If that means there's more interest in going straight to a revision-based approach (either after #1776796: Provide a better UX for creating, editing & managing draft revisions. or in parallel with it), rather than tempstore first, I think that would be ok too.

This remains one of my top 2 favourite Drupal 8 issues.

Here's an attempted re-roll from #122, back when this was passing testbot, and before things went completely off the rails.

The last submitted patch, node-preview-1510544-133.patch, failed testing.

Check watchdog, when the site title is Error there is something useful in there.

That is the most useful error it could have been. It explains exactly what is wrong, on which line, and roughly how to fix it.

EDIT: This sort of EntityInterface nonsense will go away once #1391694: Use type-hinting for entity-parameters is in.

Patch only applied with offsets. Here is a directly applying reroll. Also the fail seemed to be unrelated, due to the test system, so intrigued if this would still fail.

Note at some point there was lovely blue styling on that back link that seems to have gotten lost. Probably my fault.

IMO, the dropdown with the view modes looks terrible. I would prefer a list of buttons or links, there will rarely be more than a handful of view modes. If we have to do a dropdown, lets use one of our dropbutton thingies.

@Bojhan/@moshe: not sure of best next step there, the design in the issue summary was pre-seven-style-guide and it does not seem to look anywhere close to how Seven is evolving with http://groups.drupal.org/node/283223

Right, so the point of reviewers above was that the functionality is already greatly improved vs. the current "preview", which is a joke :) We have a passing test that works. What else do we need then? :)

This is a hack I added at some point, however, this won't fly at all as we'd have to 'fix' this for every form that alters the node form, e.g. the alias, comment settings ... Going back to the node edit form will loose data.

I've tried getting around that in #132, but got completely stuck on Form API/tempstore.

@Moshe: re #170, to get rid of EntityInterface as a generic and use NodeInterface instead, the patch does at two new functions where EntityInterface is used, however there is already 50+ uses of EntityInterface in node.module (where these 2 functions are added), and absolutely no use whatsoever of NodeInterface. So I think for consistency, it makes most sense to use EntityInterface in this patch and do a generic EntityInterface => NodeInterface conversion at a different issue.

@swentel: re #171, what do you think should be the way forward here? Rip that code out and accept the form data loss? Sounds bad. :/ What did you get tripped up with above? It is not clear from the comments, so I could use a short summary. @sun's idea in #135 to use the form cache is also an interesting one, might actually be the easiest approach?

#172: node-1510544-172.patch queued for re-testing.