-
Notifications
You must be signed in to change notification settings - Fork 356
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
Client closes transport on 'beforeunload' event even if user stays on the page #658
Comments
Related: socketio/socket.io-client#1451 The issue is in this commit: ed48b5d The @darrachequesne any suggestions on how to best handle this or a patch? Happy to contribute as well, let me know what you prefer |
This is indeed linked to ed48b5d. The problem with using I'm open to suggestions on how to fix this. |
@darrachequesne I'm not as familiar with browser specifics, but would something like a |
@zxlin Delaying the transport closing might work for your use case but it won't if the page closing is canceled by the user via the browser confirmation prompt. @darrachequesne Since I don't think there is any way to solve both socketio/socket.io#3639 and the current issue at the same time, the only solution I can think of is an option to prevent the lib from adding that It would let the library consumer decide which drawback they accept, according to their usage. A good documentation will be needed so that consumer can be well aware of the consequences. const socket = io(window.location, {
closeBeforeUnload: false,
}); |
@athouary good idea, another idea I just thought of is some libs has a pre-close event that gets fired, but if in the event handler, the user returns The user could return null for example to reject closing or be given an opportunity to send out any last bits of data before closing. Thoughts? |
@zxlin Good idea as well. It would be more flexible for the consumers to make the decision of the behavior they want right when the |
A possible workaround would be to call event.stopImmediatePropagation(), in order to prevent the library from silently closing the connection: window.addEventListener('beforeunload', (event) => {
event.preventDefault();
event.stopImmediatePropagation();
event.returnValue = 'unused string';
}); What do you think? Edited: |
@darrachequesne is this workaround supposed to work currently? I just tried and it is not working for me, the socket was still closed. On Chrome 89.0.4389.82 |
@zxlin sorry, I meant You need to declare it before the socket creation: window.addEventListener('beforeunload', (event) => {
event.preventDefault();
event.stopImmediatePropagation();
event.returnValue = 'unused string';
});
const socket = io(); |
@darrachequesne Thanks for that. I can confirm this indeed works as a workaround for me, but it seems like it's not ideal for OP's case where the logic to prompt user for navigation confirmation may not be available prior to socket creation - leading to some wonky code where one'd have to check to see if Ideally a better solution prevails for the longer term. Happy to help test alternatives. |
@darrachequesne Your workaround works very well. However, the requirement for the I'm implementing your solution as a workaround, but I still believe that an option or callback as described in previous comments would be more flexible and comfortable for the library consumers. |
Should this (new) behavior be dependent on user adding 'disconnect' event listeners on the client? Of course this would have to be documented, as adding the event listener some time later during development may suddenly break an application, but so would upgrading socket.io from previous version. |
Since [1], the socket is now closed when receiving the "beforeunload" event in the browser. This change was meant to fix a discrepancy between Chrome and Firefox when the user reloads/closes a browser tab: Firefox would close the connection (and emit a "disconnect" event, at the Socket.IO level), but not Chrome (see [2]). But it also closes the connection when there is another "beforeunload" handler, for example when the user is prompted "are you sure you want to leave this page?". Note: calling "stopImmediatePropagation()" was a possible workaround: ```js window.addEventListener('beforeunload', (event) => { event.preventDefault(); event.stopImmediatePropagation(); event.returnValue = 'are you sure you want to leave this page?'; }); ``` This commit adds a "closeOnBeforeunload" option, which controls whether a handler is registered for the "beforeunload" event. Syntax: ```js const socket = require('engine.io-client')('ws://localhost', { closeOnBeforeunload: false // defaults to true }); ``` [1]: ed48b5d [2]: socketio/socket.io#3639 Related: - #661 - #658 - socketio/socket.io-client#1451 Reference: https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event
The Syntax: const socket = require('engine.io-client')('ws://localhost', {
closeOnBeforeunload: false // defaults to true
}); |
Is there a version of socket.io-client that uses this version of engine-io-client? |
I've run into a separate issue introduced by this. Currently in Chromium browsers, clicking a link with a tel or mailto handler triggers the beforeunload event, so any time a user clicks on those, all transports on the page are closed silently. |
Thanks! I just opened a PR to fix the TS typings |
Silently closing the connection when receiving a "beforeunload" event is problematic, because it is emitted: - when downloading a file from another host Related: socketio/socket.io#4436 - when the user already has a listener for the "beforeunload" event (i.e. "are you sure you want to leave this page?") Related: - #661 - #658 - socketio/socket.io#4065 That's why the `closeOnBeforeunload` option will now default to false.
Describe the bug
Clients close the transport on 'beforeunload', even if the closing process was prevented.
To Reproduce
See my fiddle fork: https://github.com/athouary/socket.io-fiddle
Once on the page, randomly click on a blank space to make sure that the browser has detected a user interaction. Then try to close the tab. You should be asked if you really want to exit the page. Wait a few seconds, click cancel and see the error in the console:
Uncaught Error: Transport not open
.Engine.IO server version:
5.0.0
Expected behavior
If the user decides to stay on the page instead of closing it, the socket transport should not be closed. I would suggest to close it on the 'unload' event instead of the 'beforeunload' one.
Platform:
I reproed the problem on Chrome 89 and Firefox 86, with a MacOS device. I'm quite sure it does not really matter as long as preventing the page from closing is possible.
The text was updated successfully, but these errors were encountered: