"beforeunload" listener in XMPP module makes it impossible to prevent accidental tab closing

[michalsnik]

Description

It is impossible to prevent accidental closing of a tab by a user. It is due to the fact that in XMPP module there is a listener attached on beforeunload event that disconnects the user immediately when it's fired (https://github.com/jitsi/lib-jitsi-meet/blob/master/modules/xmpp/xmpp.js#L129)

I looked for similar issue in https://community.jitsi.org but didn't find one, and since this is strictly code related - I'm posting the issue here.

Current behavior

While trying to close a tab, even with custom listener that prevents default behaviour and shows up a native dialog to confirm the intent - the listener attached in XMPP module causes the connection to close, hence even by cancelling the close action - user gets back to the video conference, but being totally disconnected.

Expected Behavior

It should be possible to prevent user from accidentally closing the tab.

Possible Solution

Current listener could be updated to attach itself only on the unload event. Not sure though if that wouldn't introduce unexpected consequences.

Steps to reproduce

1. Add the following code in the application using lib-jitsi-meet:

  function onBeforeUnload(e) {
    e.preventDefault();
    e.returnValue = '';
  }

  window.addEventListener("beforeunload", this.onBeforeUnload);

2. Try closing the tab, but cancel when you see the native dialog.

3. See how you got disconnected, despite not wanting to leave the conference.

Environment details

Not relevant.

[damencho]

What if you attach your beforeunload and unload handlers prior those of lib-jitsi-meet, as pointed in the code comments?

[michalsnik]

I'm doing that actually, but if doesn't change anything unfortunately. There can be multiple listeners added to a single event, and the only way to override previous one would be to call window.removeEventListener("beforeunload", xmppBeforeUnloadHandler) with the exact reference to the handler that was attached in addEventListener in the first place - otherwise it gets executed in parallel to any other attached listener for that event.

[damencho]

Hum, interesting ... as the documentation says that preventDefault should cancel the event. Have you double-checked that yours is executed first before the lib-jitsi-meet one?

[damencho]

So we try to desperately to send the last unavailable stanza to prevent ghost participants, so removing that is not a good idea.

[michalsnik]

Yea I checked, and the way preventDefault works is it prevents the default behavior - one that might be implemented by the browser. It allows us to for example cancel submitting the form on click or opening the link when clicking on an element. Here however the already attached listener is not a default handler - it's a custom one and each one of them I believe is it's own thing and they can't influence other attached listeners by design, unless of course they have access to the listener reference, which they could then use to removeEventListener themselves.

Ghosts are also my concern, but perhaps just the unload event is fine. I might update this lib just for myself now and test the hell out of it to see how it works. It was possible with few tricks to slow it down "unload" a little bit in the past to give browser more time to close any dangling connections. But now I think navigatior.sendBeacon might also be worth looking into.

[ickarakurt]

@michalsnik Did you found any solution ?

[ghost]

@michalsnik I am also facing the same issue. Have you found any solution as of now?

[Fuzzyma]

What you probably want to try is ev,stopImmediatePropagation(). This will stop any other listener after the current one to be executed. However, I am not sure in which order listeners are executed. You probably need to bind a handler to the event before the xmpp module can do it so yours is executed before theirs

[manisar2]

Binding my custom handler before the one in xmpp module, even if it works, is not elegant always. For example, I do not want my handler to fire always on the meeting page, but only after the videoConferenceJoined event has fired. And, unless I dare to step into the jitsi code, this doesn't seem possible. I'm waiting for a good solution.

[damencho]

You can get the existing beforeunload and execute it in your unload logic, why that won't work?

[manisar2]

This is my scenario. I want the existing (jitsi's) handler to execute after my handler gets the confirmation from user. But, at the same time, I want my handler to be bound only after the user has joined the meeting (and not just on page load). This is so that if the user just loads the page that has the meeting but doesn't join the meeting, he is not bothered on navigating away.

So, I'm binding my handler in jitisi's videoConferenceJoined event (so that from this point onward, there is a confirmation needed). But this means by this time jitsi api has already attached its own handler. If I had just wanted to bind my handler on page load, I could have easily done that before sourcing jisti's js file.

But in the case explained above, I do not see any way of binding my handler before jitsi's, unless I'm missing something here.

[Fuzzyma]

Always bind it before and check for a flag you set in videoConferenceJoined. If it is set, you call ev.stopImmedatePropagation()

[manisar2]

@Fuzzyma Thanks for the suggestion. It's a much better approach, and it will work provided the beforeunload and unload events can be tamed.

No matter where I add my handler for these events (and use ev.stopImmediatePropagation()) - middle, end or at the beginning of the universe - the xmpp handler is getting fired without fail, and that kills the meeting.

Waiting for a solution. Anybody? Help.

[Jacky-Rolo]

@Fuzzyma Thanks for the suggestion. It's a much better approach, and it will work provided the beforeunload and unload events can be tamed.

No matter where I add my handler for these events (and use ev.stopImmediatePropagation()) - middle, end or at the beginning of the universe - the xmpp handler is getting fired without fail, and that kills the meeting.

Waiting for a solution. Anybody? Help.

@Fuzzyma try to set connect option disableBeforeUnloadHandlers as true, will moves all Jitsi Meet beforeunload logic (cleanup, leaving, disconnecting, etc) to the unload event.

ref:

• https://github.com/jitsi/jitsi-meet/blob/master/config.js#L419
• https://github.com/jitsi/lib-jitsi-meet/blob/master/modules/xmpp/xmpp.js#L189

you may need double check your lib-jitsi-meet version, I think this option been added recently

[Fuzzyma]

The correct solution would be to be able to completely disable jitsis automatic unload bindings so you can do it manually. That way you would have full control

[saghul]

The correct solution would be to be able to completely disable jitsis automatic unload bindings so you can do it manually. That way you would have full control

What are you missing with this option though? https://github.com/jitsi/jitsi-meet/blob/master/config.js#L419

[Fuzzyma]

The same problem people are having with beforeUnload can be applied to the unload event. So making jitsi not binding to any event and giving us the functions that need to be executed would give users the control they need for any scenario:

function myUnload () {
  // do my stuff
  console.log('foo')
  // execute jitsis stuff
  JitsiMeetWhatever.unloadHandler()
}

window.addEventListener('unload', myUnload)

I am not needing any of those atm but with this approach there will be no more issues like this ever