views:

1277

answers:

6

On a page with Ajax event, I want to disable all actions until the Ajax call returns (to prevent issues with double-submit etc.)

I tried this by prepending return false; to the current onclick events when "locking" the page, and removing this later on when "unlocking" the page. However, the actions are not active any more after they are "unlocked" -- you just can't trigger them.

Why is this not working? See example page below. Any other idea to achieve my goal?

Example code:
both the link and the button are showing a JS alert; when pressing lock, then unlock the event handler is the same as it was before, but doesn't work...?!?
The code is meant to work with Trinidad in the end, but should work outside as well.

<html><head><title>Test</title>

<script type="text/javascript">
function lockPage()
{
  document.body.style.cursor = 'wait';
  lockElements(document.getElementsByTagName("a"));
  lockElements(document.getElementsByTagName("input"));

  if (typeof TrPage != "undefined")
  {
    TrPage.getInstance().getRequestQueue().addStateChangeListener(unlockPage);
  }
}

function lockElements(el)
{
  for (var i=0; i<el.length; i++)
  {
    el[i].style.cursor = 'wait';
    if (el[i].onclick)
    {
      var newEvent = 'return false;' + el[i].onclick;
      alert(el[i].onclick + "\n\nlock -->\n\n" + newEvent);
      el[i].onclick = newEvent;
    }
  }
}

function unlockPage(state)
{
  if (typeof TrRequestQueue == "undefined" || state == TrRequestQueue.STATE_READY)
  {
    //alert("unlocking for state: " + state);
    document.body.style.cursor = 'auto';
    unlockElements(document.getElementsByTagName("a"));
    unlockElements(document.getElementsByTagName("input"));
  }
}

function unlockElements(el)
{
  for (var i=0; i<el.length; i++)
  {
    el[i].style.cursor = 'auto';
    if (el[i].onclick && el[i].onclick.search(/^return false;/)==0)
    {
      var newEvent = el[i].onclick.substring(13);
      alert(el[i].onclick + "\n\nunlock -->\n\n" + newEvent);
      el[i].onclick = newEvent;
    }
  }
}
</script>

<style type="text/css">
</style>
</head>

<body>

<h1>Page lock/unlock test</h1>

<p>Use these actions to lock or unlock active elements on the page:
<a href="javascript:lockPage()">lock</a>,
<a href="javascript:unlockPage()">unlock</a>.</p>

<p>And now some elements:</p>

<a onclick="alert('This is the action!');return false;" href="#">link action</a> &nbsp;
<input type="button" value="button action" onclick="alert('This is another action!')"/>
</body>
</html>
+1  A: 

I once achieved this goal by creating a DIV that covered the area I wanted disabled, setting its z-index higher than any of the other elements on the page, and then setting its opacity to 0. By default, this DIV was hidden by display: none, so that it wouldn't interfere with anything. However, when I wanted the area disabled, I just set its display to block.

Steve

Steve Harrison
Thanks for the answer, but that's actually not what the customer wants... ;-(The page should be shown unchanged (with all buttons, fields, etc. visible) but only prevent that the user triggers another action or the same action a second time.
ami
That's what's being suggested. There will not be any visible changes to the page. You don't even need to change the opacity - just keep the div empty and toggle the display attribute between 'none' and 'block'
ozan
Oh, yeah, I see. Should have read this more carefully ;)Will try that approach...
ami
This technique also does nothing to prevent the user from manipulating the page with the keyboard...
annakata
@annakata: Ah, yes, I didn't think of that...
Steve Harrison
+2  A: 

How about setting up a global var

actions_disabled = 0

increment when the AJAX call starts then decrement when it finishes. All your "action" handlers can then start with

if (actions_disabled) return false;

Much simpler than debugging self-modifying code!

Alternatively, to lock your controls you could set:

control.disabled="disabled"

which will have the bonus of greying them out, making it obvious to the user that they can't submit. To unlock, simply set:

control.disabled=""

NEW IDEA BASED ON COMMENTS (can't quote code in comments, it appears ...):

You can always just hang extra attributes off Javascript objects:

To lock, you could:

control.onclick_old = control.onclick
control.onclick = "return false;"

To unlock, you could:

control.onclick = control.onclick_old
Sharkey
Thanks, but unfortunately the action code is generated by Trinidad framework and I can't just alter it directly. That's why I have to use JS to change event handler code.
ami
Can you "wrap" the actions: function thingo_wrap(e) { if (actions_disabled) return false; return original_thing(e) }Also see alternative suggestion above.
Sharkey
Nice idea... I can't do that directly, but maybe use some JS again to modify the action. The reason my original solution does not work seem to be that I'm mixing up functions and Strings, right?I'll try that out. Alternatively, I could probably use a map to store original action and restore it later...
ami
Yeah, I think that probably is the problem. See my new answer ...
Sharkey
Cool... this last NEW IDEA BASED ON COMMENTS is really nice, didn't hit on that. Works fine, with small change to existing code. Thank you!
ami
No worries, enjoy!
Sharkey
+1  A: 

AJAX. Asynchronous. Just make the HTTP request synchronous. Problem solved.

annakata
Well, you're right, but... On one hand, this part of the code is generated by our Web FW and I do not have direct control on that. On the other hand, main purpose is to prevent double-submits (just as an additional layer to existing server side detection) so same mechanism should be used on all pages, not only for Ajax calls. Just mentioned Ajax as an example :(
ami
A: 

The problem with your code is a result of not coming to grips with types in javascript.

When you say:

var newEvent = 'return false;' + el[i].onclick

what this does is coerce el[i].onclick (which is a function) to a string, then concatenates it to the string 'return false;'. Then when you reassign it as so:

el[i].onclick = newEvent;

onclick which was previously a function is now a string.

Then you attempt to resurrect your old function from the string by taking a substring:

var newEvent = el[i].onclick.substring(13);

which is fine, except newEvent is still a string! So when you assign it back to onclick again, you are assigning the string representation of the original function, not the function itself.

You could use eval to evaluate the string and return the function, but please don't do that. There are a number of better ways to do this, as has been suggested by other commenters.

I would also question why you wish to use AJAX at all if you don't want to allow asynchronous requests.

ozan
A: 

Thanks guys for your ideas and answers.

Now I see that I have mixed up Strings and functions, which obviously can't work ;(

I should have made clear that we use some Web FW and tag libraries (Trinidad) which create the event handling (and Ajax) code, hence I can't edit that directly or use synchronous Ajax etc.

Moreover, Ajax is only one scenario where this code should be executed. It's purpose is to prevent the user to double-submit a page/action, which is also relevant for non-Ajax pages where you could kind of doulbe-click on a button. I know that this is not really safe, and it's only meant to be a "convenience" thingy to avoid getting the navigation error page too often (we have server-side protection, of course).

So, will try the div overlay, probably.

Thanks again,
Christoph.

ami
A: 

Using a div overlay does not prevent a user from tab-ing into your page. Usually that is OK, since most users do not tab through a page anyhow.

If you use any keyboard shortcuts on your page, they will still be available, so separate handling will be needed for those.

Alse, I assume that clicking an element that can have focus (eg. an <a> tag), then pressing enter, would still cause a double submit.

LaustN