How many times have you been posting a comment or working on a new post when you click on a link in another application and you navigate away from the content you are editing, losing all of your changes?

Well I say we introduce a script, much similiar to that of Blogger, that uses the Javascript window.onbeforeunload handler to prevent this from happening. We can set this handler to call a function that compares the contents of all forms when the page loads and then quickly compares that to the current contents just before the user is about to leave the page. If they have changed, we should prompt the user so they don't lose their work. This would save many headaches and degrade nicely for users without Javascript.

Additionally, this script should work with the newly introduced JS-based upload feature to prevent navigating away as a file is being uploaded as well.

If I have sometime I'll start work on a patch tomorrow, just wanted to get the idea into the queue right now :)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

StuartDH’s picture

Sounds like a great feature to me. We regularly get authors and editors telling us that they've lost the last hour of work by accidentally navigating away, and I've even just had one of them email me about it a few minutes ago, so it'd be great if you could put something together for this.

Cheers

Michelle’s picture

Sounds great to me, too. I often have several tabs open doing stuff and it's easy to lose my place and leave a page with unfinished edits. Would this alert you when you're going to close an unsaved tab/window as well?

Michelle

MrMattles’s picture

Title: Prevent accidentally navigating away from pages where content has changed » This is a great idea

Great for us multitaskers that rush and click the wrong thing

chx’s picture

Title: This is a great idea » Prevent accidentally navigating away from pages where content has changed

Very strong -1 as this is IE only.

praseodym’s picture

Konqueror already does this, +1 for the idea in general.

webchick’s picture

Yeah, I think chx is correc that onbeforeunload is an extension developed by MS (it's not in the ECMAScript specification. However, I found out that this is implemented in Mozilla since 1.7 (which corresponds to Firefox 1.0, I think?) and I've tested Blogger.com's version of this functionality in:

Firefox 1.0.6 (working)
Firefox Deer Park Alpha 1 (working)
Opera 8.0.2 (working)
Safari 1.3 (does NOT work)
Safari 2.0.1 (working)

So it seems to be a 'de facto' standard if nothing else.

webchick’s picture

Also, I should clarify... that "does NOT work" in Safari doesn't display any errors or anything, it just doesn't save the form results as expected (so pre-Tiger Safari users are going to be used to this). So I don't see any harm in including this functionality since it seems to degrade gracefully in non-supporting browsers.

m3avrck’s picture

Status: Active » Needs work
FileSize
951 bytes

And a patch is ready! Code was based on many example, including this and this along with adding my own thoughts and what not ;)

m3avrck’s picture

FileSize
1.54 KB

And here is the JS to throw in misc/.

m3avrck’s picture

FileSize
1.61 KB

Better JS file attached without tabs. Also, tested and working great in FF and IE6. Doesn't work in Opera 8, however no errors are present, I believe this is just because the event handler is not supported. If there is a work around for Opera, please let me know!

m3avrck’s picture

FileSize
1.61 KB

One more try with those tabs.

moshe weitzman’s picture

Status: Needs work » Needs review

hmmm. i usually find these prompts annoying. any chance we can attach the behavior only to the node and comment forms? those are the "high risk" areas. what do others think?

m3avrck’s picture

Yes, the JS is only loaded on the node edit/create pages, won't affect any admin/login/etc form. Also, the prompt is unobstrusive and if you goto save a node you don't get a warning that the contents have changed because you are explicity saving the node (hence no reason!).

Dries’s picture

I want some JS-folks to review the code.

m3avrck’s picture

FileSize
1.94 KB

After talking with Drumm on IRC, this patch makes the integration with Drupal much simpler and it's off by default. Only turned on for the node edit/change page right now but any module/form can easily add this by passing a 'TRUE' parameter.

nedjo’s picture

+1 on idea, js looks generally good, a few suggestions:

1. Instead of writing onsubmit via PHP, use a js addLoadEvent call.

2. Shouldn't the return false in isElementChanged be at the end (outside the switch block)?

3. It's be nice to find a way to make messages like 'You have unsaved changes.' translatable. Pass in global js variables via a t('') call?

4. onbeforeunload event should probably be in a if isJsEnabled test, and should parallel drupal.js's event adding (see addLoadEvent).

5. Comments should be in standard format and in present tense, e.g.,

/**
 * Checks to see if a form has been changed after the page loads
 */

6. e_ should be just e to match other js files, e.g., autocomplete.

m3avrck’s picture

FileSize
1.58 KB

Ok here is an updated JS file.

A few notes, I couldn't get the addSubmitEvent() in drupa.js to reliably work so I had to set the isSubmit() true in the PHP creation of the form. If you look at Blogger.com, they do this as well... so either we both missed an obvious way to do this, or that is the most practical. Hopefully some wise JS gurus will chime in with an answer.

Same goes for onbeforeunload event, which is completely different then addEvent defined in drupal.js.

As for the t('') I agree this would be useful but I'm not sure of the best way to do this. One way would be to put in form() a t('') passed in, and then write this to a var in formcheck.php which returns a JS file type. Thoughts?

Junyor’s picture

FWIW, Opera doesn't support the onbeforeunload event. However, this issue doesn't affect Opera anyway: form contents are retained in history as long as the page isn't closed. IOW, this feature doesn't work in Opera, but it isn't needed either.

m3avrck’s picture

FileSize
1.64 KB

Updated JS styling and JS-killswitch after talking with Unconed on IRC.

m3avrck’s picture

Just to build on Junyor's comment this functionality is built into Opera 8 and it *doesn't* produce any errors.

m3avrck’s picture

FileSize
2.1 KB

Updated patch to fix possible problem of overwriting onsubmit event handler thanks to Thox on IRC.

webchick’s picture

I tested this and can confirm that it works in both versions of Safari that I have access to: 2.0.1 (OS X Tiger) and 1.3 (OS X Panther). So looks like this code goes one better over the Blogger.com stuff, because their stuff doesn't workk in 1.3. Nice job! ;)

m3avrck’s picture

Status: Needs review » Reviewed & tested by the community

Well I'm gonna set this to commit then. Tested and working on IE, FF, Safari. Doesn't work or break Opera or Konqueror but not needed in these cases. Thox and Unconded have offered thoughts and I've modified code as needed. Doesn't seem like there is anything else left except a commit :)

Bèr Kessels’s picture

In general I dislike the feature. Konqueror somehow dies this, and that is the way it should be. Browsers should take care of this, not the web app.

But, since it is only konq. this patch has a good enough additional value :)

I would like to see this patch tested with, tinyMCE and HTMLAREA, at least. For I am quite sure this will break these modules so bad that they are near unusable.

Which brings me to the next point: using the textarea hook a simple module could take care of this.
I would very very much prefer this living in a contrib, or even a core module. Just not "enforced" on me.

And the last point: if this is for core, please use that textarea hook too. This is where that hook is for: extending the textareas.

m3avrck’s picture

FileSize
2.23 KB

Updated patch after talking with Thox and Uncloned to attach this event only to forms with a specific class (hence avoiding the problem of being on an edit page with other admin/search forms).

m3avrck’s picture

FileSize
1.67 KB

Updated JS file.

m3avrck’s picture

Berkes, this patch *does not* break TinyMCE, just tried. However, TinyMCE does not take into account this script. This should be an easy patch to TinyMCE.

Also, this doesn't affect just textareas, it affects all fields within a given form (e.g., text fields, check boxes, etc...). Sure the blunt of editing/creating a node is in the textarea, but there are still all of those other changes that can be made (new title, revision status, front page promotion, etc...) so this needs to account for them all which it does.

This script is unobtrusive and degrades perfectly well. It is tested and working in FF, IE, Safari and doens't cause problems in Opera or Konqueror which don't need this feature anyways (since they already have it).

The usability boost of this script is too enormous *not* to include in core. As a contributed module, it is really too flaky, and along those lines, autocomplete.js, inlineuploads.js and related should be in their own contributed modules :)

I do see your points and I hope this clears it up. It doesn't cause any problems and only attaches to specified forms, and is off by default. Any module can easily make use of this script when they use: form(..., TRUE). This is the same behavior as the other JS files included with HEAD as well.

Once this is in core I'll work on a patch to TinyMCE to get that up to speed ;-) (and as such, TinyMCE still works fine, no probs/errors/etc, and good reason too if you look at how the code *actually* works ;-)).

Bèr Kessels’s picture

I see. the "why" for not using form textarea is perfectly valid. And I tried to explain that, eventough I feel this belongs in the Agent, it is a usability enhancement for all the others using sillier browsers ;).
And you are right about that part of contributions, exp since you cannot use hook_texarea, having this in a contrib cannot be achieved.
I hereby retract my hesitations. (though I have no time to reapply the latest patch and test it on konq. The last JS updates broke drupal on konq, which Steven fixed, right away though!)

m3avrck’s picture

Ready to go!

nedjo’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.37 KB

Nice work m3avrck. I've made some tweaks to minimize code additions on the PHP side and improve the js. Mainly, it's now enough to set the 'formcheck' class attribute for a form. function form() will add the js file, and the needed onsubmit event will be added dynamically to forms. This is consistent with other core js files, which don't hard-code event calls but add them dynamically, based on js support.

The useful functionality is unchanged.

nedjo’s picture

FileSize
2.12 KB

The revised js file.

m3avrck’s picture

FileSize
1.56 KB

nedjo, nice! However, the addSubmitEvent does not actually set the variable true, does not work in any browser I tried, already looked into this issue. After some research I found out that Blogger.com actually sets this variable in the form element as I did. I have updated the patch to reflect this, while still incorporating all of your changes. Code is sexy and sleek, ready for commit ;-)

m3avrck’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
1.78 KB

And the new JS with the attach event taken out.

m3avrck’s picture

FileSize
1.59 KB

Fixed tab issue with patch.

nedjo’s picture

FileSize
2.12 KB

Thanks for noting that it wasn't working. The problem isn't using autoattach, it's just that I'd sloppily put formcheckIsSubmit = false in the attached function instead of formcheckIsSubmit = true. Here's the fixed js to go with update #30, using the autoattach behaviour.

m3avrck’s picture

nedjo, great catch! I totally missed that too, haha! Anyways retested, everything is still working as expected. Dries, please committ JS in #35 with patch in #30. Thanks!

m3avrck’s picture

FileSize
2.07 KB

Updated script, now you can pass in a t() string for the error message, still working great, I think this was the last issue nothing left now :)

m3avrck’s picture

FileSize
1.72 KB

And the new patch to go along with the new JS file.

Dries’s picture

Status: Reviewed & tested by the community » Needs work

Patch no longer appllies.

Not convinced this much needed feature.

m3avrck’s picture

Not convinced??? There are more than a half dozen people that agreed this was a great and useful feature.

Comments #1, #2, #3, #5, #28 where even Ber says eventough I feel this belongs in the Agent, it is a usability enhancement for all the others using sillier browsers ;) :-) And it seems there are more positive reviews of it in the thread that can be implied as well :-)

Sites that already implement this feature: Blogger.com, Gmail, Joomla (formely Mambo, demo it here: http://demo.joomla.org/ ) and I'm sure many more (I didn't go digging too much). Note, the Joomla implementation is very poor and makes it hard to leave the page. The prompt I'm using is 100% unobtrusive, can be translated, and you can click 'ok' or 'cancel'. I'd say 98% of the time this prompt won't be even be shown but when that 2% is shown (on the browsers that support/need this also degrades nicely) you'll be glad you have it. If anything, this is another easy usability win over Mambo ;-)

We could even have a new setting that says something like "enable warning for users if they are about to leave a page with unsaved changes" and turn it on that way. Would that work?

If so, I'll reroll and update patch.

moshe weitzman’s picture

Perhaps this should be a contrib module. Would be easy to set the formcheck class with form_alter(). You could even provide an admin UI for specifying which textareas are 'formcheck'

m3avrck’s picture

Hmmm is form_alter() part of the new Forms API? If so than that might work, didn't think this was possible with the current form set though.

If this is indeed the case, any thoughts for a module name? formSaveCheck? hmmm...

nedjo’s picture

After working on the patch, I found that the functionality was sometimes annoying. Many times I was prompted when navigating away was what I wanted to do. Perhaps this should be a user choice, so that particular users can choose to turn it off.

StuartDH’s picture

So that's it, the issue's binned?

I'm amazed that this isn't considered to be a valuable addition. That aren't many apps that let you make changes to a page then close (or move away) without warning you that you'll lose your work if you don't save.

If necessary, this has got to be made into a module, but I still can't see why this can't be an option in core.

Thox’s picture

I see this as an interesting optional feature. From what I've seen of the JS, it doesn't choose which forms to apply this to (e.g. blocks, search would also be affected).

Bèr Kessels’s picture

I very much dislike the route taken.
instead of:
* developing a JS way and then make it also work on non JS we should:
* Develop a HTML-only way, and then add some JS on top to enhance this even more.

For betteruplaod I ned this functionality too, be it for different reasons. My plan is to introduce a
$SESSION['edits'][node][nid] = $edit //or $node . That way we ca store all the data a user has inserted into the session, a user can go off the page, do stuff (select an image, find a term, find links etc) and when he or she returns the $node is refilled using a default nodeapi call.

I think it is smart to immediately start off with the [node] part, so that this can be expanded to more objects (users, terms, comments, files) in future without breaking the current behaviour again

moshe weitzman’s picture

Ber - your proposal doesn't solve the problem described here. This protects the user from losing his unsubmitted data in the node form. Any server side solution will fail here since the server does not know about changes in the textarea, selects, etc.

Bèr Kessels’s picture

@Moshe, I know that. For one, this is really a client issue, Safari and Konq. solve this properly themselves. But lets not leave that from getting this solution in :). I mean, if drupal can fix lacking features in clients then it should.

I am very aware that a client side solution is required. I am talking about the underlying technology. (the $SESSION). On top of that slution, we can bolt some very small JS to actually save the stuff to that $session, for example on a time based interval (for crashes) and on a BrowseAway.

But, back to the core: We need a way to save data when browsing away, Then, for example, a button [Select Tags] will save that data to session, when a user wants to go to a nice interface to select 'tags'.
This also will make the solution much broader usable. And i is hardly more work/code.
Besides that, it is much fall-back safer.

Bèr

m3avrck’s picture

@thox right now it does work with only certain forms, e.g., only the edit form and not the search box. It does this by adding a formCheck class to each form that is required, so that you can choose which forms to add this to.

With the new Forms API I plan to make a contrib module that makes use of this.

@ber your idea about a non-JS solution is quite interesting. however, this proposed JS solution solves a different problem as pointed out. once the page loads if you edit the form and then try and leave, no way for the server to know you just went to google.com without some sort of JS catch. you are write other browsers do this better than FF and IE, but for now, this is quite a solid solution, as blogger.com implements a similar solution.

Still on my todo list, the module would allow you to be able to check which forms you want this JS applied to, most likely node edit and comment create by default, but ability to add it to more too.

Zen’s picture

I personally think that Drupal would be better served with an "Auto-save draft" functionality along the lines of gmail. That would solve the basic issue of losing data due to browser crashes, connection issues etc. The problem being focused on here - that of the current browser window being hijacked because of clicking a link in a 3rd party application - is IMO not a Drupal issue and more of a browser issue. Setting Firefox to open new links in a separate tab/window (in the background) will fix this issue. Similar functionality is available for pretty much all browsers.

I've only skimmed the comments in this thread - apologies if I'm just echoing previously expressed sentiment..

Cheers
-K

m3avrck’s picture

Status: Needs work » Closed (won't fix)

Zen, I agree completely, this was merely a quick fix. In the next next release of Drupal (post 4.7), I'd like to see this feature go in as better workflow is built in that could accomodate drafts, then we can expand upon the included AJAX to auto save things.

Marking this won't fixed as a better route should be setup for the next next release of drupal.

nedjo’s picture

I've implemented this approach in a contrib module as part of the (draft) Javascript Tools package:

http://drupal.org/node/57285

m3avrck’s picture

Great work nedjo, thanks for doing this! I'll have a look and help contribute to jsTools, looks awesome!

Aren Cambre’s picture

Version: x.y.z » 8.x-dev
Status: Closed (won't fix) » Active
Issue tags: +Usability

Can this be revisited in light of Drupal's core usability emphasis? A couple of Drupal searches suggest this may not have been included in D7?

fuzzy76’s picture

This can probably be marked a duplicate of #193799: Warn before losing changes (eg: blocks and menu admin pages)

Doesn't seem to make it into 8.0 though. :(

jhedstrom’s picture

Status: Active » Closed (duplicate)

Marking as a duplicate per #55.