ericmmartin / simplemodal

A modal dialog framework for jQuery
http://simplemodal.com/
Other
506 stars 229 forks source link

Alternative IE 10 bugfix #35

Closed phawxby closed 10 years ago

phawxby commented 11 years ago

Avoids the throw and catch

henryruhs commented 11 years ago

there is no need to call s.fixIE(); if there is no support for removeExpression() and setExpression()... ?

there is a reason we have try and catch in Javascirpt and other languages, can you guarantee the script does not fail with your statement? the only benefit i see in your solution is performance.

btw: let's use typeof to check for undefined...

phawxby commented 11 years ago

I was going for performance however I discovered an IE "feature" after submitting this in that sometimes removeExpression and setExpression identify as an object, when you try and execute it obviously breaks.

henryruhs commented 11 years ago

however, let's see what eric martin will choose... he should remove the s.fixIE() at all.

jsumners commented 10 years ago

@redaxmedia your catch does nothing. Merely testing for the presence of the function, and using it if it is there, is the proper way to do it.

@ericmmartin any ideas on when this bug might be resolved?

henryruhs commented 10 years ago

@jsumners You can use phawxby solution also, I don't care.

We are waiting for merging pull requests nearly a half year and now reached IE11 - I will stop using Simple Modal because for me this project is no longer maintained...

ericmmartin commented 10 years ago

I believe this should all be resolved with https://github.com/ericmmartin/simplemodal/commit/184a26a4861c80488eeb37e3510563ad5b394987

Please let me know if you still have these issues with the most recent changes.

mikemanger commented 10 years ago

184a26a Fixes the issue for me.. although would be nice if there wasn't browser sniffing. Saying that I see that Modernizr use document.compatMode to help check for some things (e.g. hashchange) so I guess this is OK. :+1: