Warn before losing changes (eg: blocks and menu admin pages)
starbow - November 20, 2007 - 11:08
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | task |
| Priority: | normal |
| Assigned: | starbow |
| Status: | needs work |
Description
This patch fixes the usability issue where the user has unsaved changes on a page, and accidentally clicks a link or the back button and loses all of there changes. This patch uses the onBeforeUnload event to warn the user that they are about to lose their work, and gives them a chance to cancel. It currently just works on the blocks admin page, but would be easy to extend to other pages. onBeforeUnload works on Firefox, IE and Safari, but not Opera.
Note, this patch is much, much simpler that what I originally proposed over at: http://drupal.org/node/193311 (which remains a cool set of functionality, but is overkill to solve this particular problem). This patch is about 5 lines of code.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| unbeforeunload.patch | 1.69 KB | Ignored | None | None |
#1
#2
Not sure if I'm missing something but I don't notice any changes with this patch applied.
#3
Yes I missed a hard browser refresh :(
Works fine on both IE7 and FF2. I think the message is a bit too verbose though.
How about "You are leaving this page without saving your changes. Are you sure you want to discard unsaved changes?"
#4
You don't yet a whole lot of control over have the onBeforeUnload event is handled (I think it is a reaction to the years of abusing the onUnload event to popup ads). All you can do is set part of the message:
Are you sure you want to navigate away from this page?
You message here
Press OK to continue, or Cancel to stay on the current page.
OK button Cancel button
I have changed the comments in the code to say this, shortened the default message to "If you continue your unsaved changes will be lost", and made the message an argument.
#5
I made some improvements. I added in a behavior so that the popup does not get applied to forms that are loaded into the page with Ajax/Ahah. I pulled the two functions out of drupal.js into unsaved.js. And I applied the unsaved warning to the new drag and drop Menu administration page.
#6
Cleaned up comments and whitespace.
#7
Subscribe. I hope we can get this in for beta 3.
#8
Got this error against HEAD:
$ patch -p0 < unbeforeunload_3.patch
(Stripping trailing CRs from patch.)
patching file misc/tabledrag.js
Hunk #1 succeeded at 441 (offset 28 lines).
(Stripping trailing CRs from patch.)
patching file includes/common.inc
Hunk #1 succeeded at 2205 (offset 170 lines).
(Stripping trailing CRs from patch.)
patching file misc/unsaved.js
patch: **** malformed patch at line 75:
#9
If I thought there was any chance of getting this into 6 I would reroll it.
#10
I fear this is out of discussion for D6. Let's focus on implementing this for D7.
#11
I have created a new patch for D7.
#12
Patch is broken and gave me errors while trying to fix it fast.
#13
Rerolled the patch to accommodate the recent change to tabledrag.js.
#14
Fancy :D
Working fine here (FF 2.0.0.14). The Block tests still run fine. Can some more people test it on other browsers?
#15
I have tested several browsers that I have kicking around. Some work and others don't. The onbeforeunload event is not part of the official standard; it was invented by Microsoft and implemented by others.
These browsers work:
These browsers don't work:
Also, from experience I know that Safari has implemented the onbeforeunload event, so I don't expect any trouble there.
I think the bottom line is that the vast majority of users will have a better experience than before and the others will have an experience that is no worse.
#16
That konqueror is not supported is not a big problem, IMO. Konqueror is on of the few browsers that has this solved where it (IMHO) should be solved: in the client. See attached screenshot for how this is dealt (when closing a window); sorry for the Dutch locale.
#17
To the message shown: As short is sweet, there might be still room for improvement:
"Your changes have not been saved.
Do you want to save now?"
Being able to save directly from the popup would be even nicer :) - and then (but this may be a bit much to ask - navigate further in the direction the user chose before the popup came up.
#18
This functionality actually exists in my popups module, which I am hoping to bring to Drupal 7.
#19
Yeah, this might be something that's better achieved in contrib because it's not on every form that you want this to take place. It might actually be something that might do well in the Form API. Then you could perform a form_alter to change it if you wanted.
#20
@Rob: I think you misunderstood my previous comment. I was telling eigentor that the "ask to save" feature he was talking about was already available elsewhere. I continue to believe that one of the first rules of usability is "don't lose the users work", which Drupal 6 does quite regularly. I think this patch (or something like it), is important for Drupal 7 if we are serious about being kind to end users .
#21
It is my understanding that by having an unbeforeunload event handler in the page you prevent the page from being cached. That includes any Javascripts and server-side scripts. While this may be desirable for forms, this side-effect - if indeed true - needs to be considered.
#22
Briefly tested. Appears to work well.
Screenshot inlcuded for easy reference.
#23
So, why dont we give people the option to actaully save there changes? As that is likely what the user wants to do, they have to click cancel, and scroll to click save?
#24
If this goes in, it better have an option to disable it pretty damn quick.
Option #3: Never show this pop-up again!
Unlike the target audience for this 'fix' I actually know how to drive a browser, and if I click Back or Refresh it's because I want it to behave like it's supposed to. When I leave a page without saving changes, it's almost always because I don't want to save the changes. People who have a habit of doing otherwise clearly have never used webmail, and are in need of educating.
The idea itself is not silly, and may even be a useful requirement in some deployments. I'm not objecting to the concept outright. I'd object to it getting in my way. If it can be done as a contrib option, that's where it belongs.
#25
I completely agree with dman's comment #24. Not only do I think that this should be a user option, as dman suggests, but I also think that it should be able to be turned on and off via site configuration. It should also be easy to overwrite the text, whether it be through a theme function or tpl file.
#26
Comment 24 has it right. It allows users to choose for themselves what behaviour they want, the first time they encounter a page with this behaviour.
I would suggest something like:
"This page has unsaved changes:
(X) Save changes
( ) Continue without saving
[X] Always do this when leaving the page"
( ) denotes radio button, [ ] denotes checkbox.
While the wording isn't necessarily the best, the general idea is there. It solves the usability problem for people who are currently having troubles with those pages forgetting their changes, and supports users like dman who understand how it works. The setting could also be part of a users's settings maybe?
I don't fully agree with comment 25 however - it shouldn't be settable at a site level, rather it should be settable at a user level, as many sites may have several admins all with different usage patterns.
#27
The onbeforeunload event provides an opportunity to display a message to the user indicating they may lose data if they navigate away from the webpage.
While you could certainly construct a custom warning message by creating dom elements and attaching them to the document, keep in mind that JavaScript is not multithreaded. Any events that are received during the execution of the onbeforeunload event handler are stored in the event queue and will not be immediately available while the thread of execution is in the event handler. Also, when the onbeforeunload event handler function exits the browser will either display a message (giving the choice of whether to cancel or continue to the user) or continue loading the new page (the next event will be the onunload event). Any custom dialog used here would not be very useful because we would never receive the events from the user's selections.
Therefore, the onbeforeunload event handler is not a viable opportunity to do a custom dialog. You can certainly save the information in a form, in a cookie, send it to the server, etc. But for the browsers that support onbeforeunload, the behavior is fairly hard coded. The browser constructs a message from the text that you optionally provide combined with its own hardcoded message and the user chooses to continue or cancel.
My point is that we cannot change that dialog. We can display a bit of text in the dialog, but we can't prevent the browser from putting its own explanatory text in the dialog. And we can't display our own dialog to handle that particular event. There are limits to this event that we aren't going to be able to get around.
The suggestion that we could provide a way for the user to indicate that they do not wish to see the dialog again is a good one, though there is no way to add this choice to the dialog box that will appear. We could provide a user setting that is configured elsewhere that would control whether this dialog is used or not.
#28
paul.lovvik : I get your point, but then we can't do this. We can't go around introducing bad dialog boxes, with that creating another usability problem. Try to find a way - is the only suggestion I can give.
#29
Marking it not a requirement for Drupal 7.
#30
Is there anything like this for 6?