added beforeunload confirmation during active meeting#16933
added beforeunload confirmation during active meeting#16933Adarsh765-jpg wants to merge 3 commits intojitsi:masterfrom
Conversation
|
If this is to land, it must be configurable and disabled by default, since that is our current behavior. |
|
Hi, thanks for your contribution! |
|
@saghul thanks for the feedback. I'll update the implementation to make it configurable and disabled by default. |
|
@saghul I’ve set it as the default now, and it can be enabled from Settings → General if someone wants to use the feature, just like you suggested. Let me know if anything needs tweaking or if i can help with some other issues. |
|
@saghul waiting for approval |
|
|
||
| if (typeof window !== 'undefined') { | ||
| window.addEventListener('beforeunload', beforeUnloadHandler); | ||
| if (configEnabled || settingEnabled) { |
There was a problem hiding this comment.
should also check for undefined, which is falsey, but you likely want the user setting to take precedence, if set.
There was a problem hiding this comment.
@saghul I've made the changes. Please go through it.
- Set default enableBeforeUnloadConfirmation to undefined - Replace manual OR logic with getPropertyValue - Maintains precedence order: JWT > URL > Settings > Config Implements review feedback from saghul.
|
@saghul Could u please review my new changes? |
|
Any news for approval? |
|
@saghul @farapholch still waiting for the approval |
|
any news? @saghul |
|
Have you tested this? As it seems to me it will not work. |
|
There is another PR trying to do the same #17052 and there they discovered that it will not work with changes only in jitsi-meet. |
This PR adds a beforeunload handler when a conference is active to prevent accidental tab closure or refresh disconnects.
Implementation:
Testing:
Fixes #16928