[drupal-devel] [feature] Prevent accidentally navigating away from
pages where content has changed
m3avrck
drupal-devel at drupal.org
Tue Sep 13 17:32:28 UTC 2005
Issue status update for
http://drupal.org/node/30220
Post a follow up:
http://drupal.org/project/comments/add/30220
Project: Drupal
Version: cvs
Component: base system
Category: feature requests
Priority: normal
Assigned to: Anonymous
Reported by: m3avrck
Updated by: m3avrck
Status: patch (ready to be committed)
Attachment: http://drupal.org/files/issues/drupal_18.patch (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
Previous comments:
------------------------------------------------------------------------
Thu, 01 Sep 2005 22:47:42 +0000 : m3avrck
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 :)
------------------------------------------------------------------------
Sat, 10 Sep 2005 23:48:33 +0000 : StuartDH
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
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:33:58 +0000 : MichelleC
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
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:34:19 +0000 : MrMattles
Great for us multitaskers that rush and click the wrong thing
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:36:52 +0000 : chx
Very strong -1 as this is IE only.
------------------------------------------------------------------------
Mon, 12 Sep 2005 14:38:40 +0000 : praseodymium
Konqueror already does this, +1 for the idea in general.
------------------------------------------------------------------------
Mon, 12 Sep 2005 15:07:21 +0000 : webchick
Yeah, I think chx is correc that onbeforeunload is an extension
developed by MS [1] (it's not in the ECMAScript specification [2].
However, I found out that this is implemented in Mozilla since 1.7 [3]
(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.
[1]
http://msdn.microsoft.com/workshop/author/dhtml/reference/events/onbeforeunload.asp
[2]
http://www.ecma-international.org/publications/standards/Ecma-262.htm
[3] http://www.mozilla.org/status/2004-03-01.html
------------------------------------------------------------------------
Mon, 12 Sep 2005 15:09:15 +0000 : webchick
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.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:10:18 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/node.module_11.patch (951 bytes)
And a patch is ready! Code was based on many example, including this [4]
and this [5] along with adding my own thoughts and what not ;)
[4]
http://www.codestore.org/store.nsf/cmnts/451FA051398A9AF486256EE0000FB7D1?OpenDocument
[5] http://www.blogger.com/app/scripts/formcheck.js
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:10:45 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck.js (1.54 KB)
And here is the JS to throw in misc/.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:22:56 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_0.js (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!
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:24:18 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_1.js (1.61 KB)
One more try with those tabs.
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:32:48 +0000 : moshe weitzman
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?
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:42:39 +0000 : m3avrck
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!).
------------------------------------------------------------------------
Mon, 12 Sep 2005 21:53:01 +0000 : Dries
I want some JS-folks to review the code.
------------------------------------------------------------------------
Mon, 12 Sep 2005 22:09:10 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_15.patch (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.
------------------------------------------------------------------------
Mon, 12 Sep 2005 23:38:35 +0000 : nedjo
+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.
------------------------------------------------------------------------
Tue, 13 Sep 2005 13:54:35 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_2.js (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?
------------------------------------------------------------------------
Tue, 13 Sep 2005 14:06:44 +0000 : Junyor
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.
------------------------------------------------------------------------
Tue, 13 Sep 2005 14:15:56 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_3.js (1.64 KB)
Updated JS styling and JS-killswitch after talking with Unconed on IRC.
------------------------------------------------------------------------
Tue, 13 Sep 2005 14:39:15 +0000 : m3avrck
Just to build on Junyor's comment [6] this functionality is built into
Opera 8 and it *doesn't* produce any errors.
[6] http://drupal.org/#comment-44066
------------------------------------------------------------------------
Tue, 13 Sep 2005 15:04:37 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_16.patch (2.1 KB)
Updated patch to fix possible problem of overwriting onsubmit event
handler thanks to Thox on IRC.
------------------------------------------------------------------------
Tue, 13 Sep 2005 15:13:59 +0000 : webchick
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! ;)
------------------------------------------------------------------------
Tue, 13 Sep 2005 15:25:21 +0000 : m3avrck
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 :)
------------------------------------------------------------------------
Tue, 13 Sep 2005 17:13:48 +0000 : Bèr Kessels
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.
More information about the drupal-devel
mailing list