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.

Comments

starbow’s picture

Status: Active » Needs review
catch’s picture

Status: Needs review » Needs work

Not sure if I'm missing something but I don't notice any changes with this patch applied.

catch’s picture

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?"

starbow’s picture

Status: Needs work » Needs review
FileSize
1.88 KB

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.

starbow’s picture

Title: Warn before losing changes (eg: blocks admin page) » Warn before losing changes (eg: blocks and menu admin pages)
FileSize
2.78 KB

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.

starbow’s picture

FileSize
2.86 KB

Cleaned up comments and whitespace.

BioALIEN’s picture

Subscribe. I hope we can get this in for beta 3.

birdmanx35’s picture

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:

starbow’s picture

Category: feature » task

If I thought there was any chance of getting this into 6 I would reroll it.

Pancho’s picture

Version: 6.x-dev » 7.x-dev

I fear this is out of discussion for D6. Let's focus on implementing this for D7.

paul.lovvik’s picture

FileSize
2.88 KB

I have created a new patch for D7.

R.Muilwijk’s picture

Status: Needs review » Needs work

Patch is broken and gave me errors while trying to fix it fast.

paul.lovvik’s picture

FileSize
2.88 KB

Rerolled the patch to accommodate the recent change to tabledrag.js.

R.Muilwijk’s picture

Status: Needs work » Needs review

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?

paul.lovvik’s picture

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:

  • IE 6.0 on Windows 2000
  • Firefox 3.0 on Windows 2000
  • Firefox 2.0.0.15 on Ubuntu
  • Firefox 3.0 on Ubuntu
  • Flock 1.2.3 on Ubuntu (based on FF)

These browsers don't work:

  • Opera 9.5.1 on Ubuntu
  • Konqueror 3.5.9

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.

Bèr Kessels’s picture

FileSize
72.35 KB

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.

eigentor’s picture

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.

starbow’s picture

This functionality actually exists in my popups module, which I am hoping to bring to Drupal 7.

RobLoach’s picture

Category: task » feature

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.

starbow’s picture

Component: javascript » usability
Category: feature » task
Priority: Normal » Critical
Status: Needs review » Needs work

@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 .

illuminaut’s picture

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.

ultimateboy’s picture

FileSize
113.45 KB

Briefly tested. Appears to work well.

Screenshot inlcuded for easy reference.

Bojhan’s picture

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?

dman’s picture

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.

ultimateboy’s picture

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.

nigel’s picture

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.

paul.lovvik’s picture

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.

Bojhan’s picture

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.

Bojhan’s picture

Component: usability » base system
Priority: Critical » Normal

Marking it not a requirement for Drupal 7.

rukaya’s picture

Is there anything like this for 6?

Tor Arne Thune’s picture

Version: 7.x-dev » 8.x-dev

Still a valid issue after the release of 7.0. Moving to 8.x. I have had a relative suffer from this, as she clicked "More information about text formats" and lost all the changes she had made. She thought that it would open in a new tab or pop-up. I think this is one of the most frustrating things that can happen in Drupal, and anywhere else on the web for that matter. I highly recommend and cheer on a solution for this.

RobLoach’s picture

I think this could become part of the Form API itself. Having it embedded straight into tabledrag.js and drupal_add_tabledrag() seems like a hack. Maybe something like...

  // Display default warning text when attempting to navigate away.
  $form['#warnwhennavigatingaway'] = TRUE;

  // Display some custom text warning when attempting to navigate away.
  $form['#warnwhennavigatingaway'] = t('Dries kills a kitten every time you loose your changes.');

We could also lean on existing code if we wanted too:

eigentor’s picture

@1V yeah that is a baddie. Should be seen as a "newbie Trap" and get an entry in the Drupal UI Hall of Shame.

Wtower’s picture

Subscibe. An absolute must.

nod_’s picture

Assigned: starbow » Unassigned
Issue tags: +JavaScript

Should be using javascript events instead of adding something to tabledrag/form.js. We can use sessionStorage, so that'll help quite a lot :)

nod_’s picture

Version: 8.0.x-dev » 8.1.x-dev
Issue summary: View changes

I don't see that happening for 8.0.x

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.