0

So I started off with the error because e.preventDefault doesn't work in IE7. I found event.preventDefault() function not working in IE on here and implemented the answer - the problem is that where

event.returnValue = false; 

seems to work for everyone else there, it doesn't for me! I also put in

if (event.returnValue) {
    alert();
}

And got no alert. Here is my code

$('#share a').click(function(e){

   if (event.preventDefault) {
        event.preventDefault();
    } else {
        window.event.returnValue = false;
    }

    if ($(this).parent('li').hasClass('email')) {
        window.open(this.href,'share-this','height=750,width=500,status=no,toolbar=no');
    } else {
        window.open(this.href,'share-this','height=400,width=500,status=no,toolbar=no');
    }
}

When I click the link I get an Invalid argument error in IE7 on the ternary operator line. I know it is hitting event.returnValue = false; because I put an alert in there to debug.

Any ideas?

Community
  • 1
  • 1
Joe Beaver
  • 111
  • 5
  • 1
    It's unclear to me, if this code is in an native style attached event handler function, or is it in an jQuery-attached function? If the latter, `event.preventDefault()` should work in older IEs too. – Teemu Jul 05 '13 at 11:20
  • Please notice also, that [jQuery_2.X+](http://jquery.com/browser-support/) versions are not supporting IE < 9. – Teemu Jul 05 '13 at 11:55
  • @Teemu I have updated the code to the full code, don't know why I didn't put it in there first time around. I am using jQuery 1.8.3 – Joe Beaver Jul 05 '13 at 13:51
  • I'm not good at jQuery, but it seems that you can simply put `e.preventDefault();` to your code, and forget all the stuff around it... Notice `e`, not `event`, since you've passed `e` as an argument. If I'd recall correctly, `window.event` in older IEs is "alive" only in an event handler function, and it can't be cached and passed to another function, like jQuery does in its eventhandling model, though its properties can be passed forward... – Teemu Jul 05 '13 at 13:56
  • When I first started with this problem I just had `e.preventDefault();` and that was what was causing the error. The code had only got the if statement there after I found the post linked in the OP. – Joe Beaver Jul 05 '13 at 14:04
  • Odd... Anyway, now you're passing `e` and checking `event.preventDefault` instead of `e.preventDefault`... – Teemu Jul 05 '13 at 14:11
  • Yeah, because checking for `e.preventDefault()` errors on IE7 and `event.preventDefault()` doesn't. Gotta love it. – Joe Beaver Jul 05 '13 at 14:13

2 Answers2

1

Ternary operators don't work like that. This statement:

event.preventDefault ? event.preventDefault() :  event.returnValue = false;

Is invalid. You should use:

if(event.preventDefault)
  event.preventDefault();
else
  event.returnValue = false;

Ternary operators work as r-values, not as general if/else statement replacements.

Also, for IE < 9 you should use the global window.event (window.event.returnValue).

Update

Actually, ternary operators seem to work like that in Javascript (so it's not invalid). It's still a "cheat" according to the ECMA specifications and IMHO, it hurts readability of the code (for programmers who can distinguish between lvalues and rvalues, at least).

Jcl
  • 27,696
  • 5
  • 61
  • 92
  • Hi, I tried with this first off, it still doesn't work. I get the Invalid argument on line 955 which is `event.returnValue = false;` in the else block. – Joe Beaver Jul 05 '13 at 10:58
  • Try `window.event.returnValue = false` for IE < 9 . I'll update the answer – Jcl Jul 05 '13 at 11:02
  • 1
    @Jcl What's exactly wrong with that ternary? Why you think it's invalid? This works well: `1 ? alert('1') : alert('0');`. – Teemu Jul 05 '13 at 11:04
  • I still get the same Invalid argument error with `window.event.returnValue = false;` :-( – Joe Beaver Jul 05 '13 at 11:08
  • @JoeBeaver The last line in Jcl's answer is probably the key issue. `event` is probably a jQuery-normalized local property here, use `window.event` when you refer to IE's global event object. – Teemu Jul 05 '13 at 11:09
  • @Teemu, the expresions on the ternary operator should be assigment expressions (or, rvalues), not statements. It actually may work on javascript because javascript is pretty loosely checked -basically, the weirdest things work-, but even if it does work, I wouldn't say it's a good use of it (you can check the specs [here](http://www.ecma-international.org/ecma-262/5.1/#sec-11.12) ) – Jcl Jul 05 '13 at 11:13
  • @JoeBeaver not sure then, it *should* work, but I have no IE < 9 here to test, sorry... just answered off my head :/ – Jcl Jul 05 '13 at 11:14
  • No problem, hopefully someone will think of something. – Joe Beaver Jul 05 '13 at 11:15
  • Just another quick thing to try: are you naming the parameter passed to your event (generally called `e`) `event`? If so, it might be using the local parameter. That should not be the case when using `window.event` but I'm wild guessing here... javascript on IE 7 and below had tons of quirks. – Jcl Jul 05 '13 at 11:20
  • @Teemu seems the ternary operator does indeed work like that... still it's a bit of cheating the language (I wouldn't go as far as calling it _obfuscation_, but almost) – Jcl Jul 05 '13 at 11:24
  • @Jcl I agree with your edit what comes to readability : ). But cheating, I don't know... Every executed statement returns a value, even statement like `2;` returns 2, or a given identifier: `variableName;` (providing `variableName` is declared). The return value is not just used anywhere. – Teemu Jul 05 '13 at 11:27
  • @Teemu, yeah well, I'm probably too old for a language where you can write a statement like `2;`. I guess I'm used to more strict languages :-) – Jcl Jul 05 '13 at 13:01
  • @Jcl: `2;` is a valid statement in languages that are older than you are, like C... – Elias Van Ootegem Jul 05 '13 at 13:24
  • @Jcl The parameter passed into the event was named e, then I was using window.event so shouldn't have used the local. I hate ie7 =] – Joe Beaver Jul 05 '13 at 13:46
  • @EliasVanOotegem "i'm too old for" is a way of speaking in Spanish, it doesn't necessarily mean age or years. Sorry, thought it was the same in English – Jcl Jul 05 '13 at 19:01
1

Your code looks as though you're using jQuery. If you're handling an event that was bound using a jQ function, like on, delegate or bind, jQ will wrap the event object for you. You should have access to jQ's preventDefault(), stopPropagation() and stopImmediatePropagation() methods, rendering your ternary irrelevant.
If you're binding the event yourself, make sure the event variable isn't undefined:

elem.onclick = function(e)
{//e should contain the event object, except for IE<9
    e = e || window.event;//old IE's set the event as a property on the global object
};

If you find this a ropey, and who can blame you, you could augment the Event.prototype. I do this for code that needs to support old versions of IE, and it hasn't let me down yet...

The ternary operator can't be sure how to evaluate the statement event.preventDefault ? event.preventDefault() : event.returnValue = false. You might mean for the JS engine to interpret the code as (event.preventDefault ? event.preventDefault() : event.returnValue) = false (assign false to the result of the ternary) or (event.preventDefault ? event.preventDefault() : event.returnValue = false) (treat the entire statement as a ternary expression).

To avoid this ambiguity, group the assignment expression, to let JS know that the assignment is the or part of the ternary:

event.preventDefault ? event.preventDefault() :  (event.returnValue = false);

Having said that, using a ternary as a statement is generally considered to be bad practice. a far more common way of writing your code is:

e.returnValue = false;//returnValue is set on Chrome and, I believe FF, anyway
//if some browser Event objects don't have this, there's no harm in adding a property
//if the event object is read-only, the assignment will fail, silently
if (e.preventDefault)
{
    e.preventDefault();
}
Community
  • 1
  • 1
Elias Van Ootegem
  • 74,482
  • 9
  • 111
  • 149
  • I have updated my code in the OP so it has the full function, don't know why I didn't put it in first time around. I tried adding in `e = e || window.event;` and changing `window.event.returnValue = false;' to `e.returnValue = false;` (if that is what your suggestion was) but once again, I still get the error. – Joe Beaver Jul 05 '13 at 13:56
  • @JoeBeaver: your callback function is expects is passed the event argument, which you called `e` (`.click(function(e)` <--. This means that `event` will probably be undefined, so `event.preventDefault` is the same as `undefined.preventDefault`, which throws an error (`cannot access property of undefined`). Change `event` to `e` in your code, except for `window.event`, because in IE, the global object event reference is called `event` – Elias Van Ootegem Jul 05 '13 at 14:31
  • with this code in place: `if (e.preventDefault) { e.preventDefault(); } else { window.event.returnValue = false; }` I am still getting an error (Invalid argument) on the line `window.event.returnValue = false;` – Joe Beaver Jul 05 '13 at 14:48