Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xdr-streaming connection closed on "javascript:;" in IE8 #90

Closed
tomaszdurka opened this issue Oct 3, 2012 · 16 comments
Closed

xdr-streaming connection closed on "javascript:;" in IE8 #90

tomaszdurka opened this issue Oct 3, 2012 · 16 comments

Comments

@tomaszdurka
Copy link

We're having an issue with xdr-streaming transport under IE8 while using <a> elements with href="javascript:;".

Reproduce steps:

  1. Download example: https://github.com/sockjs/sockjs-node/tree/master/examples/echo
  2. Run server, open http://localhost:9999 - should work fine.
  3. Add following code into index.html body: <a href="javascript:;">Break connection</a>
  4. Reload page - Connection should be established and work normal
  5. Click newly added <a> element
    Result: Connection is closed.

Actually any javascript: ... code within href attribute will break the connection. Is this known issue? We can workaround it, but would be great if we could keep those hrefs as they are right now.

@majek
Copy link
Member

majek commented Oct 3, 2012

Wow, IE will never stop amazing me. I can reproduce that in IE9. Frankly - I doubt there is a quick workaround. I guess that clicking on a triggers some form of page reload mechanism, that cancels outgoing XDR requests. Only later IE realizes that there isn't a page refresh really.

The same seem to happen to 'iframe-xhr-polling' transport. I guess XDR / XHR just gets killed when user clicks on javascript: link.

I'm not sure if we can do anything about it - sorry. Maybe try using onclick on a elements instead? Dirty workaround idea - jquery rule to match all a elements and just execute the javascript code but from onclick handler?

There seems to be related SockJS bug - SockJS doesn't seem to trigger onclose handler in this case, that looks wrong.

@tomaszdurka
Copy link
Author

Thanks for quick reply.
We've checked iframe-htmlfile transport. Seems to be the same problem.

We see the same problem when using Faye.
Only Socket.IO doesn't suffer from it, seems they work around it somehow.

@Yaffle
Copy link

Yaffle commented Oct 3, 2012

cant reproduce with IE8 and with XDR

@majek
Copy link
Member

majek commented Oct 3, 2012

@tomaszdurka Socket.io uses flashsockets on IE by default, right? You're sure you're testing when it's on XHR/XDR transport?

@majek
Copy link
Member

majek commented Oct 3, 2012

@Yaffle You're sure you get messages echoed back after clicking the link?

@Yaffle
Copy link

Yaffle commented Oct 3, 2012

@majek, i tested with XDomainRequest with my own page and server, not with SockJS

@tomaszdurka
Copy link
Author

Tests were based on socket.io chat example. We've tested both htmlfile and xhr-polling transports - both work fine.

@majek
Copy link
Member

majek commented Oct 3, 2012

@Yaffle Thanks, that was actually quite helpful.

@tomaszdurka Okay, it is SockJS misbehaviour. Quick fix: find this line in sockjs.js and comment it out:

utils.attachEvent('beforeunload', unload_triggered);

Explanation: SockJS actively cleans up connections on page reload. This is in order to avoid memory leaks in some browsers (firefox is know to leak websocket objects, IE leaks pretty much everything). Apparently javascript: links trigger onbeforeunload event on window. SockJS cleans up state on this event. Thus this bug.

@majek
Copy link
Member

majek commented Oct 3, 2012

The online demos use released SockJS version, so no, you can't test it
online. You need to get the compiled javascript from
http://cdn.sockjs.org/sockjs-0.3.js , modify as I instructed and use
that in your code, at least until I release new version.

@tomaszdurka
Copy link
Author

Yes, we have commented that out and works for xdr-streaming well. Thanks for this fast hotfix.
However even after commenting it out there is still the same issue for iframe-htmlfile transport.

Thanks again.

@majek
Copy link
Member

majek commented Oct 3, 2012

Are you sure the iframe is using the modified sockjs.js file? You need to change that on the server side (see sockjs_url option passed to sockjs-node).

@tomaszdurka
Copy link
Author

You are absolutely right. Am quite new to SockJS and completely forgot about this property.
I wasn't aware it actually builds the iframe with this url as script source.

Confirmed - this fixes the issue for iframe-htmlfile as well. Thanks, am looking forward for the release where this fix will come live then.

@tomaszdurka
Copy link
Author

Hey Marek,
I was wondering if you are able to provide some permanent solution (fix) to this issue. Or the quirks listing is all you want or going to do.

I've been looking into the socket.io code and they seem to attach that onbeforeunload event handler only if some conditions are met (CORS and !XDomain) - or when sync disconnect on unload option is enabled.
https://github.com/LearnBoost/socket.io-client/blob/master/lib/socket.js#L52

Maybe this can point some more flexible (to us) solution. If you not plan to implement any adjustments please let us know what is your reasoning.
Thanks,
Tomasz

@majek
Copy link
Member

majek commented Oct 8, 2012

Thanks for the socket.io link, interesting.

I think I'll drop attaching to onbeforeunload at all. The main reason for using this event was due to quirkyness of opera iframes, AFAIR. I think it's fairly safe to drop this event in 99% of cases. I may write some special case for opera if it still is a problem (opera 12 was released recently), but I doubt it will manifest itself.

IE: I believe it's fairly safe to remove the line I suggested earlier.

@konklone
Copy link
Contributor

Seems like this is close-able?

@Yaffle
Copy link

Yaffle commented Jun 26, 2013

the note "Don't use "javascript:" links on a page that uses SockJS. For some reason clickling on this type of link breaks XDR/XHR requests on IE (see #90)." should be removed from README

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants