Many ways to lose data on form input in the overlay

I hate to be "that guy", but again I have to question what exactly the benefit of including the overlay module in core (and as a default behavior to boot), rather than as a contrib module. With all of the critical issues lingering for D7, does it really make sense to throw this unused/untested module into the mix at such a late date? If this was so badly needed by users there would be something similar and heavily used in contrib, and yet there is not.

If this was so badly needed by users there would be something similar and heavily used in contrib, and yet there is not.

Drupal 7 made something like the overlay vastly easier. It would have been much much harder to do cleanly on D6.

Losing data with the overlay is a problem. If automatic saves are out of order for D7, I'd do a confirmation "Do you really want to close the overlay? You have unsaved content." whenever escape is hit, the close button is clicked or a link that would leave the overlay is clicked.

Drupal 7 made something like the overlay vastly easier. It would have been much much harder to do cleanly on D6.

Yes, and Drupal 4.7 made something like CCK easier, which is why it went straight into core, right? I'll ask again: why is this module in core? It seems like a very big mistake to include this in core at this point- there are well over 300 critical bugs left in the Drupal 7 queue- shouldn't we be focusing on those rather than trying to fix something that is (most likely for most users) not even needed?

Please keep core vs. contrib discussion at #659488: Properly test the overlay to determine if it belongs in core or contrib. Thanks.

There was talk in the original overlay issue about adopting the approach from the Dirty Forms module, which pops up an alert (using the onbeforeunload event) if you've changed the contents of a form. That would make sense to me. We'd want to be able to use the same solution either with or without the overlay, though, I imagine.

I remember there was an issue somewhere to show a confirmation dialog when you entered data into a form and want to move away ore click the overlay close button.
Cannot find it anymore now...

Maybe a good idea, would have to be gauged to overly nagging when people voluntarily want to lose their data. Thus off-switch would be good idea.

You can't make a difference between "oops I hit close" and "I want to close without saving". Personally, I think the last doesn't happen that much...

It would be best to get our ideas from popular services, like gmail. There, it saves a concept every x-seconds (if there is something to save), and if it contains unsaved stuff before you navigate away, it will throw a warning. Works really good for me.

However, the idea of having the possibility to save to a concept is something totally new. I can't check right now, but I don't think drupal 7 has such a thing as concepts, which means we could introduce it ourselves in the overlay module (doesn't make sense to me), or save it in javascript variables, which means we could at least save the data if the overlay get's closed, but obviously not if there is an refresh, or if a non-overlay page is requested.

Now that we have jQuery UI Dialog in core, we could pop up a ui dialog with three buttons:

Save and Close| Close without Saving | Cancel

There probably are situations when getting this dialog repeatedly would be annoying, but that is far better than a client losing 30 mins of work to a mis-click.

Add an option to the dialogue: "Don't show this message again" and of course a backend switch to turn it back on if someone changes his mind. This provides an easy fix for people who feel overly nagged... Especially with not-so-experienced users it happens s a lot that they lose their data. And those ard the quickest to leave drupal and never come back again...

What would the dialog switch on. Is 'onbeforeunload ' browser safe? Would it trigger off the overlay iFrame?

Also, would a way to make this generic for all forms be to hash all the form values on page load, then check if the hash has changed when they try to close?

#7 Save and Close| Close without Saving | Cancel

How is the use case where the user restores context with the Browser's back button handled?

Save | Cancel

What would be the difference between "Close without Saving" and "Cancel"?

cancel should read "Continue Editing"

(Adding tags)

After reading this interesting article why warnings never really should be used, it got me thinking of this issue.

Besides the usability issues of using a warning in general (see link), I absolutely do NOT like the above solutions. While the reasons behind them are good, the idea of having to look for an option to toggle to warning does not sound good. Either it cloggs up the screen if you want to have the toggle option in your face, or it would be too many clicks away.

The only thing that would really be an improvement is the gmail approach, that is autosave content periodically to a draft, save the draft again when hitting the back button (cross browser mess to intercept the back button event by the way). After navigating away from a page, a drupal message of the type 'warning' should be shown if there are still unsaved drafts.

But where to save these drafts? A javascript variable is not a sound solution, because those get destroyed when the page get's refreshed. So it should be saved to the database, probably in a separate table containing all the content entered and somehow contain a reference to the page. I'm not really sure how this part should be implemented (not exactly a php/db guru), thoughts/ideas/brainstorming would be great! I was thinking of simulating the submit of a form, serialize the array of the submit, and save that. No clue if that is a good idea at all.

So to summarize:

1. Autosave periodically unsaved content to a draft table, unsaved content could also be selected options for radiobuttons and checkboxes... Forms in general.
2. Add a tab to the overlay called "Drafts" if there are drafts (which should lead to a page containing an overview of all drafts).
3. Try to autosave content to draft when onunload()/onbeforeunload() (-ish, the aforementioned cross browser mess)
4. Always generate a warning message if there are unsaved drafts after a page refresh. This should cover the case where the overlay is closed (and thus can't see the "Drafts tab", and to generally get more emphasis on the unsaved drafts.

It's a pretty big change I know, but really, shipping the overlay without some sort of decent, usable dataloss prevention would be terrible. Using warnings would be a step backwards.

#14 Kiphaas7

I think that dataloss prevention in general is a wonderful feature. However, under the principle of graceful degradation, surely this solution would apply to all pages, and not just overlay?

What would be the difference between "Close without Saving" and "Cancel"?

alexanderpas is right:

cancel should read "Continue Editing"

The difference is one is closing the "warning", and the other is closing the whole overlay.

#14: I wholeheartedly agree. Warnings are never ideal and gmail's model is pretty good. Much tougher to implement though (as you pointed out). There is already kind of a concept of drafts in that you can have unpublished revisions. This only accounts for nodes, however. There are other forms within the overlay that would be annoying to lose data on. Settings forms, block config, etc. Drafts don't make sense for these. So we have a general issue of a form in the overlay, where the user has made changes but has not saved. While I agree that nobody likes warning popups, I can't think of a better solution that accounts for the variety of situations.

From the perspective of the content creator, autosave feels a lot less fragile than the pop-up, and perhaps both should be combined.

We should remember, I think, that email isn't necessarily a good precedent for posting to a blog. I've got users that will spend an hour writing a post, adding links and polishing their prose, and if there's a glitch, I'm going to hear about it, as I should. Posting is very much a form of self-expression, in a way that email is often not, and so the loss of content that you poured your heart into feels a lot more painful than loss of data. Therefore, anything that makes the content creation process in overlay feel less fragile is to be applauded.

#15:
I'm not really sure what you mean by graceful degradation, I'm used to the term used in the context that the page should still be usable if javascript is disabled. So I'm guessing you meant that it should work even with the overlay closed? I agree, but for the first implementation, I'd rather not do it, since it raises quite a few new problems, like, where to put the code. Should it live in it's own module? Where should the link to the drafts be without the overlay? Should the autosave be used on all forms? (probably not, think of voting in a poll and not submitting, autosave should not fire there).

By keeping it restricted to the overlay, we can leave the selection of forms up to the overlay since overlay only opens administrative links. However, the functions for autosave should indeed be made so that it would be relatively easy to port them to a much broader use.

Keep in mind that we're discussing this in the first place because overlay borked browser cache: If you enter data on a normal page, and hit a link per accident, hitting back will will return you to the previous page with your unsaved data from the browser cache. Just because we're trying to fix and simultaneously improve the caching of unsaved data for overlay does not necessarily mean there is a need for this on normal pages. It would also involve much bigger changes for HEAD (some already mentioned in previous lines), and I'm not sure if that's allowed at this stage in development.

It would be nice if we could get an opinion from people like webchick or Dries.

#16:
I don't think we should rate solutions by how easy they are to implement :). I already gave a (rough) possible solution for forms in general in #14? Quoting myself is never a good idea :), but anyway:

But where to save these drafts? A javascript variable is not a sound solution, because those get destroyed when the page get's refreshed. So it should be saved to the database, probably in a separate table containing all the content entered and somehow contain a reference to the page. I'm not really sure how this part should be implemented (not exactly a php/db guru), thoughts/ideas/brainstorming would be great! I was thinking of simulating the submit of a form, serialize the array of the submit, and save that. No clue if that is a good idea at all.

If we have the entire submitted form array in the database, and the link to the page where the form resides, it should be possible to rebuilt the choices/data made with javascript, or am I missing something?

alexanderpas is right, indeed I still think that 2 buttons are better than 3, so maybe "Cancel | Close" would make more sense; in the end save should be already in the overlay.

#14 "A javascript variable is not a sound solution" but maybe a cookie with some encrypted dump of the form would be a client side (with all drawbacks) only solution; not really a "draft" but could solve the issue: after all I'm only expecting to save the data in my browser here and now only if I click something wrong.

#19 The drawbacks of that are huge. Even with encryption, flat file storage in general (and especially form values) leads to huge potential security risks, or at least that's what I always was told. Also, I don't see why we should not try to improve the caching for overlay (since improvement is possible), instead of just emulating the normal behavior.

#18 Yes, that's what graceful degradation means.

Out of curiosity, are there any other modules in core that do not permit graceful degradation? (One could argue that this is OK, since it's only overlay that needs autosave in the first place, but autosave is also a feature that content creators would welcome. See comment at #17 on the difference between data and created content above.) And are there other modules in core that have functions that do not gracefully degrade, and are "relatively easy to port" to "normal pages"... How? In contributed modules? Seems confusing.

As to where the code for "normal pages" should live, presumably in the overlay module. Where else?

An autosave feature would be excellent for Drupal 8. It's way too big to get into Drupal 7, which has an alpha being released in two weeks. It can be solved in contrib, however (where there's already an autosave module).

I think the best we can hope for at this point is an onbeforeunload alert, building on the work done in the dirty forms module. Even that isn't going to happen in core unless we get some code written pretty quickly.

This feature is based on the onBeforeUnload event of the window object which, at the time of writing, is only supported by IE4+, Mozilla Firefox, Google Chrome. Those using other browsers (or when javascript is not enabled) won't be warned when leaving dirty pages. Ah, c'est la vie!

Safari and Opera are both considered A Grade browsers, having a solution in core that is not cross browser would be a bad dx decision and a huge wtf.

Besides that, nagging people with a popup is a really bad ux decision, even if you can disable it. If the choice is lose data or get nagged by an alert when about to leave the page, I'd rather lose the data.

Either popup or just data loss, overlay would be a broken implementation. I reluctantly have to agree that an autosave function is too late with only 2 weeks to go before alpha. Too bad considering overlay is primarly for content editors, and those are the ones that would need this fixed (properly) the most.

Sure an autosave would be better. But if it is to save the user from losing a lot of content and being nagged, I'd prefer being nagged.

While this may be not the best approach: People know it from everywhere. Almost every desktop software tries to keep you from losing your content like this. And most of the time you cannot even switch it off. Everyone hates it, but everyone is used to it.
But the main point is: What everyone hates a lot more, is losing half an hour of work. This is what makes you drop your monitor off the balcony.

But saying "If we cannot have the best possible solution, we won't do anything at all" bases on a wrong estimation where we are. We are trying to iron out really bad UI problems. We won't get anywhere near polished and clever before D8.

So while I hate nagging popups: here is a first stab: ugly but impossible to ignore :)

(1) Does this pop up when the user navigates off the page using the browser's back button?

(2) What happens when I click the [x] in the modal dialog? Is that the same as "Cancel"?

(3) Does "don't show again" apply by session, or is it part of the user's preferences?

1: Good Idea, did not even think of that. Would this be possible with Javascript?

2: Yes, [x] is a replacement for cancel, saves us one button and is in sync how these nag-boxes work e.g. in Microsoft Word or Photoshop

3: I would propose make it a user's preference, since it must be permanent. If I throw the salesman out and shout "Never come back" I would not expect him to say "Well you did not say Never applied to anything else but yesterday" if he harasses me again the next day.

I'm confused. if [x] is a replacement for Cancel, then why do we have two user interface elements that do the same thing? And does the [x] in this modal dialog mean the same thing as the [x] in the overlay modal dialog beneath?

Let's not dismiss onbeforeunload so quickly. The whole reason it exists is that the back button popup described in #25/1 is only possible using the onbeforeunload event. It is supported in Safari, and Opera theoretically saves form contents in history, so wouldn't need a popup (although I haven't tested that with the overlay). The downside is that we don't get to style the dialog box. You get a browser native popup (see http://www.4guysfromrolla.com/demos/OnBeforeUnloadDemo2.htm for an example). That means we can't add a "Don't show this dialog again" checkbox. But frankly, if we're not going to protect users from the back button, I don't see the point in adding the feature at all.

The tricky part of this -- no matter what kind of dialog box we use -- will be determining when a form is "dirty" and when it isn't, and which forms should trigger the behavior, which is why I suggested that the dirty forms module is the place to start, since a lot of that work has been done already. Another thing to note is that this would need to play nicely with any contrib autosave feature.

@ksenzee
I just read a few posts about onbeforeunload, and my understanding is that we would have to use the browser supplied alert box? Is this only true for IE?

[Edit] Just realized your previous post basically states exactly that.

we would have to use the browser supplied alert box? Is this only true for IE?

To my knowledge it's true for all browsers.

@ksenzee: Well the real high-end-solution would be Autosave, sure. And we will not have it for D7. So what can we do without it.

I do not fully agree about the back button: While some (not too many) Websites and applications (say e.g. Googledocs) have Autosave, the vast Majority of Web Forms do not have it.

If a user enters Data into a form and clicks the back button, he loses this Data in I'd say more than 90% of the cases. I'd say a normal web user expects this and thus will not click the back button. So I'd be fine with leaving the back button being a bad guy and rather styling the form and having the possibility to put the "Don't show this dialog again" checkbox in it.

Having some kind of popup will save new users a lot of grief, now we have to decide where to make the tradeoff.

#32 David writes:

Maybe I'm naive, but I don't expect this at all. Every modern browser that I know about handles this situation correctly, at least in general. If you navigate away from a form and hit the back button, the form is still filled out.

I agree. In fact, when I'm writing, I tend to have multiple tabs open, multitasking, and I've often navigated off a Drupal post that's not yet saved by accident. And when I return, I absolutely do expect the data to be saved.

If overlay is making typical browser behavior worse, that's a big issue and needs to be fixed.

OK, i never noticed Drupal tends to lose data more than other web applications. Probably am doing too much Drupal and have become so paranoid I always save longer texts to a notepad somewhere.
For more experienced users trying to navigate back to the form would be an option. For the not-so-webby I'd guess once they left the form they won't find back anyway or panic and start to swear anyway :)

But this would be two seperate issues: Fixing Drupal's erroneous losing content on the form should be fixed.
Still I see the topic of this issue rather in preventing the user to navigate away from a filled out form at all, which is the safest option to prevent data loss.

> For the not-so-webby I'd guess once they left the form they won't find back anyway or panic and start to swear anyway :)

Which sounds funny until one of your users has lost an hour of work and, if you're lucky, only asks you for support, or if you're not lucky, passes the bad karma on to you. (And as for me, I'm very paranoid, and still can lose data. That's why it's especially important that Drupal no make a bad situation worse.)

Not critical. Please read and understand Priority levels of Issues, thanks.

Solution: Disable Overlay.

I am not sure about this, you are saying "Disable:overlay" but thats not a solution to a critical bug. Obviously losing data in the content creation form, is critical - since that is the most likely usecase of Drupal.

Just because you can disable a module, doesn't mean that critical bugs in that module aren't critical. That's especially true for stuff that's enabled in the default profile and targets first time users.

At the risk of stating the obvious, something like #24 would be a huge usability win. If we did implement the "Don't show this dialog again" checkbox, would it be a global or per-user choice?

All other methods of losing data aside (cancel, X, back button), having the overlay close when the user presses the escape key is troubling to me. I am a screen-reader user (primarily JAWS for Windows). One of the often used keystrokes with JAWS is CapsLock (or Insert) + Escape. This stroke is used to refresh the virtual buffer, which is a virtual document that is a textual representation of the DOM, and the way that web pages are read and navigated 99% of the time. The escape key is also used to take JAWS out of the mode used to enter information into a form field. The actual keystrokes and how this all work are not necessary to understand, it is sufficient to understand that escape is used more frequently by some users of some assistive technology products. On occasion I will end up hitting escape before Insert, or accidentally on it's own when not required.

I have lost the overlay (and sometimes data in the overlay) enough times that I have disabled the module. When a user presses, or activates, a control like X, cancel or Back, they are at least in some way aware that something negative may happen, when a user inadvertently presses escape (be they a screen-reader user or not) they deserve to be prompted about whether closing the page and losing their data is really what they intended.

See related: #716604: Escape key closes overlay dialog without requiring confirmation.

Yes, Escape key making you lose data is troubling me too. When I press the back button on my browser accidentally or even close the tab I've been typing in, my browser usually saves me (because it restores the form input when I go forward again or press ctrl-shift-t to reopen the just closed tab). When typing in the overlay, though, and pressing Esc by accident, my input is lost. That sucks.

I propose to make the Esc press show a confirmation dialog first (with focus on the "Really close" button, so that Esc followed by Enter closes the overlay without having to move the mouse).

The Esc key is a much easier to deal with issue than the other problems. There is no need to add a "nag" message. The Esc key (or clicking [x]) should hide the overlay, but not remove it from the DOM yet. At the same time a non modal message should be added to the screen that says "Unsaved data was discarded: Undo". The undo link un-hides the overlay, and the user keeps editing. This can be applied to all overlays. Of course, navigating away form the page or opening another overlay does discard the data, but the user has already been told that his data was discarded, so it is expected behavior.
If we cannot do auto-save, we can do this. I am not sure how this works with screen readers. The undo message can either be added to a message div at the top of the page (like gmail) or it can be an absolutely positioned bar at the top of the screen, etc. Of course this will ultimately be decided by theme designers.

@servantleader

I'm not sure how screen-readers will handle the non-modal dialog either, but arguably better than the modal dialog that is the overlay itself, dialogs and screen-readers is an issue that causes confusion for even some of the most experienced accessibility developers. There is an open related issue where discussions specifically about accessibility and dialogs related to Overlay can take place, so as not to distract from the main conversation here #716612: Overlay is not accessible to screen reader users.

Thanks servantleader for the suggestion in #42 - that makes good sense, and I hadn't thought of it quite that way. We even have a UI design we could use for the non-modal "You have unsaved data" message - Mark and Leisa originally had a blue ribbon bar below the toolbar that was intended for this kind of thing. Let me see if I can come up with an implementation that's simple enough for this late in the cycle. I doubt I can do it without breaking string freeze, but maybe we can get yet another overlay exception...

The blue bar seems like a good way to handle this. For reference, there are screenshots of the original concept at http://www.d7ux.org/edit-on-page/. I think it was originally conceived with respect to in-context edit links, but the ribbon bar should work well for this use case.

@ksenzee: we're not in full string freeze yet, that's going to be closer to RC (i.e. when there aren't known critical bugs which might have to break strings like this one), so as long as there's a good reason for stings changes it shouldn't be a problem.

@ksenzee

First thought is that sounds like a reasonable approach to take for assistive technology users. Might be ideal to set focus to the non-modal dialog when it appears so that AT users are aware that it has appeared.

What I mean by this is that a screen-reader / magnifier (perhaps others) user may not realize that the dialog is present, depending on where the focus naturally lands when the overlay dialog closes.

@Everett: We can definitely set focus to the message. It won't be a dialog actually - it'll be a full-width div at the top of the page, just below the toolbar - but same principle. I'll test it in NVDA as best I can before I post a patch.

I am well known as a negative kind of man so you can dismiss my comment entirely if you want, besides it does not help to boot, but I feel like you are trying to solve an unsolvable problem (in an elegant and intuitive way).

The root of the evil (to me) seems to be : "overlay" is a modal concept and it introduce artificial state.

First Modal dialogs are generally seen as bad UI design for many reasons , but especially because nobody knows what happens when you do "something else" and with tabs in browsers, you can loose tab focus and loose application focus (application switch) and what the user is expected to implicitly understand that will make him feel good about its "data half-typed in the overlay" when he switch or some other bad designed application take the focus on its desktop or mobile phone ?.
No arm done when you are browsing picture (but even "pictures browsers overlays" are inconsistent in what they do when you click outside the "frame" or press ESC) but when you are inputing datas ????

Second : modal introduce layered context/state, and that context as a local implicit meaning that is sometime and often opposite to the general context. That's the trouble with the "X" close button : does it mean save ? cancel ? and it should not mean the same in the overlay's overlay dialog box ("you have unsaved work ...") were "cancel" is of dubious meaning ...).

All of this troubles for what gain ? I think the problem goes goes well beyond "ready for core" or not, it is, in my book, one of these "fashionable" false good ideas. I am quite sure I would turn them OFF ASAP in any Drupal 7 installation. And I could take bets than Drupal 8 will not have them because they will look so "cliché" and "dated".
I don't want to dismiss the work that is done (especially because I am not doing much to help), and I am sure overlays can be a good and sexy addition to some use cases but certainly not the general use case.
I understand that the more one invest in something the more reluctant one is to "throw" away or pull back the stuff or reduce its scope, that's the only reason I wrote this even knowing my voice matter very little, for good reasons ....

PS: the more I think about it the more I think it is a bad solution to one big real Drupal problem : its lack (I would even say refusal) of containment and hierarchical context : that means lacks of context for "actions" witch then leads to the problem overlay are trying to solve : when you do some action you can't be easily brought back to the place you were initiating the action. That applies to many user actions when adding content, working with lists, and administrative actions : when I am adjusting sub feature D of A(admin root menu)->B->C->d I am never brought back to C but A, unless the developer provided me special shortcut links to C (but never B since B is rarely part of the module C->d are the administrative part of)... but if he does , then what happens when I "apply" the changes on page D... but I digress.

Please keep discussions about whether the overlay belongs in core at #659488: Properly test the overlay to determine if it belongs in core or contrib. Thanks.

Annakan: I think this post of yours is interesting. Please participate in the discussion at #659488: Properly test the overlay to determine if it belongs in core or contrib. The major aim of the overlay is indeed to keep the context the user was in when he initiated a task. But let's not sidetrack this issue here.

I'm *really* loving that blue bar. Forget modals and popups on top of overlays. Use the bar!

+1

As a trigger, we could compare strings from formSerialize(), to see if anything has changed. If anything has changed: blue bar showing a message "you have unsaved content".

However, I do see issues with pages that have multiple forms, should we display what form has unsaved content? (More importantly, how..)

---

I still think an autosave option is a possibility, at least for nodes in overlay! Since ajax submit has gone into core, the only thing we want to do is force an ajax submit every x second, while making sure the title is prefixed with [AUTOSAVE] (or something like that), and that the content is not published.

Thoughts?

We will not add autosave to D7, that might've been possible if #282122: D7UX: "Save draft" and "Publish" buttons on node forms had gone in, but that issue was completely derailed months ago.

ksenzee: is there a patch to test on this yet? I'd love to test a patch on this... :)

subscribing. as a vi user i'm in the habit of hitting escape frequently and this one bit me while doing some testing.

Rolled this quick patch which unbinds the escape key from the jQuery modal dialog used for Overlay. This is a jQuery modal dialog config option.

+1 for #58

We're doing our own keybinding in overlay-parent.js, actually, so we'd need to remove that. But yes, definitely, if I run out of time and can't finish the patch I proposed in #44, we can just remove the ESC keybinding before release.

+1 to #58 !

I just wanted to add that the Overlay module is the first module I disabled when installing DPA5. It's annoying IMHO and offers me nothing special, sorry.

In preparation for the new "major" issue priority, I'm tagging this (not actually critical, but still important) issue as priority-major.
See: http://drupal.org/node/669048#comment-3019824 (#669048: We should have four priorities not three (introduce a new 'major' priority)) for more information about the coming major issue priority.

Closing overlay on ESC keypress is removed in #668640: Overlay shouldn't be based on jQuery UI Dialog. So there is currently no patch that needs work/reviews.

#62 Aforementioned patch also introduced usage of 2 iframes; a new page is loaded into the inactive iframe. It might be possible to refactor the code a bit so usage of the back button reactivates (making it visible) the previous iframe without actually reloading its content.

We could use http://api.jquery.com/detach/

Also we should investigate how we can prevent the hashchange handler (Drupal.overlay.eventhandlerOperateByURLFragment) reloading the iframe content when using browser's history buttons. Maybe FF doesn't even fire the hashchange event when using these. This seems to me the cause of different behavior in different browsers.

Here's one solution to the problem. We store all the unsaved form data in a giant javascript variable, then restore the data to the forms as they reappear in the overlay. This relatively simple patch does that, and should fix the problem of restoring the data in the overlay. Limited testing done in Firefox 3.6.3 on Ubuntu.

$form.find(':input').each(function() {

There are easier ways to do this. Also, you're missing textareas and select elements.

http://api.jquery.com/serialize/
http://api.jquery.com/serializeArray/

Or using the jQuery form plugin (assuming this is still included as it was in drupal 6)
http://jquery.malsup.com/form/#api ("formSerialize" section)

Personally, I'd go for serializeArray(). :)

Second note: how to handle wysiwyg, since those replace the textarea with an iframe. The method of storing data in a variable needs to be hookable by modules.

Just as a note, I am not missing textareas and select elements. http://api.jquery.com/input-selector/. Using serializeArray will require us to eval() the result as it returns a string. If we want to go for serializeArray anyway, I'd be happy to reroll the patch tonight. However, if we want to include a hook for modules to save form data, it might be easier to just stick with the current approach.

Correction, I was confused, .serializeArray() will work fine.

So serializeArray won't work for us because it will not store any value for, for example, a checkbox that is not checked. This is no good because we need to explicitly store that the checkbox is unchecked so we know to uncheck the checkbox when the form reloads. If the data for the checkbox is missing, we need to assume that the checkbox wasn't there before, due to, for example, changing settings or enabling new modules in the interim, and leave it alone. I intend to leave that part of the code as-is. Leaving as needs work pending the change to allow other modules with complex widgets to hook into the form save/restore process.

Added an event trigger for data saving/reloading.

Patch looks good.

- Missing ; at end of Drupal.overlay.saveFormData function definition.
- Instead of putting the code directly into Drupal.overlay.loadChild I would move the code to overlay-child.js as an Drupal.overlayChild.behavior.

Then wouldn't the saved data be lost when the child iframe closes? I'd have to do tricks with parent.Drupal.overlay or something? It's definitely possible, I'm not sure why that's a better way to do it, though.

And I apologize for the missing semicolon, nice catch, will fix that up tonight.

Duh.. you're right.

Another issue I found is that closing the overlay by opening a non-administrative page will remove the formData.

Patch rerolled. The patch does not support reloading form data when you actually change the main Drupal page, at that point the JavaScript variable is gone and there's no easy way to get it back. Note that if you hit the back button on firefox, it will still have the form data there with or without this patch. So, since I see no easy or clean way to keep the data saved across multiple page requests (really, do we want to deal with invalidating the saved data there anyway?), I have not - nor do I intend to - included that feature in this patch. This patch is nevertheless a huge improvement over existing behavior, and I believe it should be thoroughly considered in its own right. If someone else would like to propose or code a way to save, load, and invalidate form data across multiple page requests, please, by all means, feel free to do so. :) Only functional change in this patch is the addition of the semicolon.

#69: Gah, I didn't remember that from the :input selector. Granted, the name is confusing :).
#71: The argument that it would be more consistent for other modules to use this code instead of serializeArray() sounds good to me.

Looking only at the code in the patch (didn't try it yet), it looks good, except that you're not caching $(this) in both .each() functions. Caching saves you a lot of unnecessary jQuery lookups.

+    $form.find(':input').each(function() {
+      var name = $(this).attr('name');

Change to:

+    $form.find(':input').each(function() {
+      var $this = $(this);
+      var name = $this.attr('name');

(And offcourse change every $(this) to $this further on.)

Needs work as per review in #77

Modified #76's patch using #77's advice.

This is not a release showstopper so lowering priority from 'critical' to 'normal'.

Changing to major as per tag.

Tag update

#79: 655388_79.patch queued for re-testing.

Reroll.

Moved code into dedicated functions storeFormData() and restoreFormData() and called them from within dedicated event handlers.

I would have set this issue to RTBC if I hadn't worked on the patch...

Micro optimization, and probably just being obnoxious (but that's me :) ):

+        // For radio buttons, only store the value if this specific radio button
+        // is currently selected.
+        else if ($input.is(':radio')) {
+          if ($input.is(':checked')) {
+            self.formData[formId][name] = $input.val();
+          }
+        }

Could be written as:

+        // For radio buttons, only store the value if this specific radio button
+        // is currently selected.
+        else if ($input.is(':radio:checked')) {
+          self.formData[formId][name] = $input.val();
+        }

Kiphaas7, radio buttons that aren't checked would be handled in the else statement.

Huh? I'm checking for both radio and checked in the first "else if" loop, instead of checking for radio in the "else if", and then check for checked in the "if" loop.

:radio:checked should only return checked radios, correct? So I didn't change any behavior?

Kiphaas7, but :radio elements that are not :checked would then fall back to the else statement. So yeah, it does change the behavior. This would break the implementation for radio buttons.

Patch doesn't apply anymore?

#85: overlay-formdata.patch queued for re-testing.

This patch proves (again) that we're trying to resemble and write a new web browser with Overlay.

This patch miraculously makes Overlay work with the back button again. If this goes in maybe i don't have to warn people away from Overlay.

Sorry, this needs review from some JS folks, ideally ksenzee.

subscribe

This is at least a "major", IMO. What a terrible problem.

Subscribe

Don't think it was mentioned before:
http://diveintohtml5.org/storage.html

Could be nice to provide some progressive enhancement on top of the current patch by adding a persistent local storage for those browsers that support it.

I never tried it but there's http://drupal.org/project/node_edit_protection that may provide some ideas, too. Latest patch is #85.

#85: overlay-formdata.patch queued for re-testing.

Rerolled.

I finally found time to review this patch. It definitely stops the back button bleeding, and I'm fine with it from a technical perspective. My primary concern is this scenario:

1. I get in a bad mood and go start a blog post about how much I hate life.
2. I decide midway through the second paragraph life isn't so bad after all.
3. I close the overlay, or navigate to the front page, or something.
4. I decide to write a blog post about cute kittens.
5. I hit node/add/blog and the page gets prepopulated with the remains of my "I hate my life" post.

The only way I can get rid of that old form data is by refreshing the page, right? And nobody is going to know that trick except people who have read this issue. Browsers handle that situation natively by treating forms differently depending on whether you got there via the back button or via a fresh click. And I'm not aware of any way to determine whether we got to node/add/blog via the back button or a click, except for the new history state events in HTML5.

Maybe webchick or the UX team can opine on whether this scenario is more or less of a WTF than what we have now.

In drupal 8, I hope we will know more about the context of the page view.

It seems that we just need to discern the intent of the author. Do they want to create a new post or modify a previous draft or unpublished node?

So it seems that the proper workflow should be that if I invoke the action for creating a node I should create a node .
If I start creating a node and go away from the page without saving I should be warned that doing so will discard my work.

@ksenzee: That makes a lot of sense. But, what if this would be the scenario. Might be a bit of over engineering:

1. I get in a bad mood and go start a blog post about how much I hate life.
2. I decide midway through the second paragraph life isn't so bad after all.
3. I close the overlay, or navigate to the front page, or something.
4. I decide to write a blog post about cute kittens.
5. I hit node/add/blog and the page shows an empty form, with a warning/message stating something along the lines of: "An unsaved entry called I hate my life was found. If you want to continue with that entry, please click here."
6. If the user chooses to replace "cute kittens" with "I hate my life", store the cute kittens story, swap with "I hate my life", and change the message to unsaved content of "cute kittens". So the user can always go back in case of an accidental click.

So, instead of assuming that the user always wants to continue, assume that the user always wants to start something new. Show a warning or message if an old unsaved entry was found in memory which, when clicked, prepopulates your form (while saving any content entered so far).

This system could then be enhanced by making smarting assumptions with the history state events.

I like that idea. It could be unobtrusive like the "Do you want to save your password" header bar in firefox, and would give the user the option of restoring form data without doing it automatically, which I see could potentially be very confusing.

I like the suggestion in #107 very much. The workflow Katherine describes at #105 is exactly the snag I hit when I was testing webform on Drupal Gardens the other week, and I can say from experience that it was extremely jarring and confusing. It also would've required me to undo a bunch of stuff I'd done before. In a node with just a title/body field that's not too big of a deal, but in a node with 10+ fields it's really annoying.

If we went one better and added a "cancel" link to the bottom of the node form, we could make the choice to abandon "I hate my life" even more explicit. This decision should probably be discussed in another issue (probably exists somewhere already, too).

@110: I'm personally not convinced that the Views situation (and therefore ui pattern) is comparable with the situation here. The core issue here is that the overlay loses content more easily than normal pages (including views).

If my memory serves me right: Views gives you a warning that the content you're working with _right now_ is unsaved, it does nothing with that unsaved data if you navigate away.

Anyways, all patches here are probably only stopgaps until drafts/concepts are in, which would make it possible to just autosave to a draft version. Although I'm curious how validation (and consequently errormessages) would be shown for autosaved drafts...

Hm. That's true about Views's existing pattern.

I guess another approach could be a dsm() at the top that says "This is content from the Article I hate my life, created on 2011-02-12 23:34:30. If you would like to discard this content and start a new article you may cancel this entry." or something like that.

I guess my main point is please don't make me undo everything I did in the previous posting to make a new one, and please don't pre-populate stuff without telling me wtf just happened. :)

except for the new history state events in HTML5

Surely the ultimate would be to emulate the usual browser behaviour as closely as possible
As HTML5 is one of the top 5 initiatives of Drupal8 (http://buytaert.net/html5-in-drupal-8) if there is an HTML5 method to reliably detect forward/back vs link/etc. can we not first make use of this before falling back on another method

I did some research and there is not in fact any way to detect forward/back vs link even in HTML5. You can control the history by moving people back and forth, but you can't read it.

I think that the scenario in #107 makes more sense for node forms, at least, than the Views-like proposal from #110/#112.

Nodes are content, views are configuration - it seems to me that it is more likely someone would want to start fresh with content, whereas configuration you want to save at all costs.

I guess the problem then becomes to decide which case is more common when people use the overlay - I would guess that creating nodes is probably more common though, since I suspect 90% or so of site administrators don't use the overlay to carry out their tasks. It seems like the overlay module should cater to the needs of a less skilled audience, which is what #105 & the response in #107 hint at.

Recent post about History API here : http://dev.opera.com/articles/view/introducing-the-html5-history-api/

I'm hoping that this issue can be simplified by the addition of a more verbose the HttpContext. But for the time being couldn't we have a urls that look like this:

http://mysite.com/
?c=browser-back (if the browser back event is triggered)
http://mysite.com/
?c=browser-forward (if the browser forward event is triggered)

the API I liked to above seems to allow for the custom crafting of urls based on triggered event with :

window.history.pushState (craft a forward event)
window.history.replaceState (decorate a back event)

Thoughts?

Re #117: No such luck. See #115. You can control the history when someone clicks a link, but you can't read it. That is, you can't detect when someone arrives at a page via the back or forward button versus when they arrive at a page via a clicked link. I actually spent about six hours trying every way I could think of to read history using the HTML5 history API and came up blank. I would love for someone to post some proof of concept code to prove me wrong, but until that happens I think we have to assume we can't detect the back button.

Re #115, #117 and #118:

History API fires the popstate event whenever a new entry is added to the history, or when is navigated to an existing history entry. The popstate event fires for both the back & forward button, but also for a elements. Wouldn't simply binding popstate and $(window).bind('click', ... be enough to differentiate between the general popstate and the more specific clicks on links?

As in: popstate should always fire for moving through the history (or adding a new one), while clicking on a link is always about adding a new one. That should mean when both events fire, a click on a link was found, and when only popstate fires, the back or forward button was used.

I couldn't quickly make a proof of concept proving my thoughts though, so there might be something seriously wrong with this train of thought...

Surely the ultimate would be to emulate the usual browser behaviour as closely as possible

I'm just wondering: wouldn't it make more sense to take content editing out of the overlay, and put it back into the browser where it belongs, rather than engineer a new way to prevent people from losing data when they press escape or some other button, just to keep editing in a javascript UI? The original spec was, afaik (and as far as such a thing exists), to allow users to perform certain admin functions "in context", but what context are we helping with by pushing people to create and edit their content via a javascript ui, rather than in the browser? If I click on "create content" I am switching context, so there's no reason for the overlay in that situation. I know that this wouldn't solve the cases where users were working with admin screens such as views and content types, but it's far less annoying to lose a few minutes of config work than 30 minutes of writing/linking/editing.

I'm with Alex UA on this. I've been overriding overlay's admin paths and setting them just for more admin-oriented pages, confirmation dialogs, etc. (using it as a modal dialog, mainly, rather than a complete and total "anything that's not involved in simply viewing content" device. It works quite well for that.

Tagging issues not yet using summary template.

This issue has no direction, I feel like we are trying to fix 6 problems at once. What is the problem? Is it overlay unique?

No, this issue existed before overlay.

The problem is "clicking a link on a form should not obliterate form values without warning" or perhaps "links that provide more help should open in a new window rather than destroying form values."

"Make it so that clicking the back button works automatically behind the scenes and does not destroy any form values. For the overlay, that might not be possible (based on discussion above), at least not without a massive refactoring such as #885690: Overlay reversed."
For this solution, how is it going to work? Wouldn't I loose content if I kill that tab or session? That way, I feel the second option is more what the user is use to. Also, while we warn the user if they would like to save their content (or have the auto-save feature), we should inform the user where the content is saved.

Of course, this is a known issue but it is always good to back it up with data.

One of the major findings of the WYSIWYG usability study (Drupal Gardens but relevant to Drupal) was:
Why did I loose my content?
While trying to look for ways to customize the toolbar, one participant left the editor to realize that his content was lost. He expected it to auto-save or prompt if he wanted to save the document. Although only one of eight participants encountered this issue during the study (partially because of the structure of the tasks), it is a critical issue owing to severity of the problem.

@David Thanks for the summary.

I wonder, whether we should pursue a partial fix for this in Drupal 7 by creating for example a modal dialog for the filter tips and warn (alert dialog). Or fix it properly by having auto-save and potentially re-factoring. I have done extensive testing on this particular thing, for a different system (an application on document management) - and often found the "are you sure you want to quit" message very distracting, because people are not used to an browser alert box on the web but also because it was often triggered when they wanted to leave for a different task.

Modal popup for filter tips is at #87994: Quit clobbering people's work when they click the filter tips link and is a quagmire all its very own.

#104: overlay-formdata-655388-104.patch queued for re-testing.

Tagging.

re #131: Another scenario where data can be lost in the overlay can be found here: #1542472: Clicking on multiple anchor links while in overlay causes a page refresh potentially causing form data to be lost ... I'm not sure wether to mark that issue as a duplicate of this one. I think they are separate and I know that the scenario described in that issue has not been discussed here, but it may be that the work here would solve that issue anyway. Thoughts?

#1542472: Clicking on multiple anchor links while in overlay causes a page refresh potentially causing form data to be lost is marked as a duplicate

#58: Close-Overlay-Escape-Key-655388.patch queued for re-testing.

Re-rolled the patch on #104.

Overlay is dead to D8 #2088121: Remove Overlay.