[drupal-devel] [feature] Prevent accidentally navigating away from
pages where content has changed
m3avrck
drupal-devel at drupal.org
Thu Sep 15 22:59:48 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_19_0.patch (1.59 KB)
Fixed tab issue with patch.
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.
------------------------------------------------------------------------
Tue, 13 Sep 2005 17:32:25 +0000 : m3avrck
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).
------------------------------------------------------------------------
Tue, 13 Sep 2005 17:32:55 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_4.js (1.67 KB)
Updated JS file.
------------------------------------------------------------------------
Tue, 13 Sep 2005 17:45:21 +0000 : m3avrck
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 ;-)).
------------------------------------------------------------------------
Tue, 13 Sep 2005 17:54:37 +0000 : Bèr Kessels
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!)
------------------------------------------------------------------------
Thu, 15 Sep 2005 16:31:11 +0000 : m3avrck
Ready to go!
------------------------------------------------------------------------
Thu, 15 Sep 2005 20:50:06 +0000 : nedjo
Attachment: http://drupal.org/files/issues/formcheck.patch (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.
------------------------------------------------------------------------
Thu, 15 Sep 2005 20:50:44 +0000 : nedjo
Attachment: http://drupal.org/files/issues/formcheck_5.js (2.12 KB)
The revised js file.
------------------------------------------------------------------------
Thu, 15 Sep 2005 22:52:08 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/drupal_19.patch (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 ;-)
------------------------------------------------------------------------
Thu, 15 Sep 2005 22:52:39 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/formcheck_6.js (1.78 KB)
And the new JS with the attach event taken out.
More information about the drupal-devel
mailing list