-
-
Notifications
You must be signed in to change notification settings - Fork 829
Prompt for ICE server fallback permission #3309
Conversation
Added @nadonomy for design review, as I made up this text myself, so perhaps he has some better wording. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm. Do we need terms of service/privacy policy on the fallback TURN server?
what doesn't lgtm is that the tests are broken for |
Ah yeah... 😓 I'll look around for the right place to make the tests work around this. |
cancelButton: _t('Dismiss'), | ||
onFinished: (confirmed) => { | ||
if (confirmed) { | ||
cli.setFallbackICEServerAllowed(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intended to also be saved as a device setting.
I wasn't sure either. After some review, since the only data exposed to the fallback STUN server, and that is called out explicitly in the permission dialog, it has been agreed that we don't need additional documents for this case. |
This stores the ICE server fallback permission in a device setting so it is remembered across sessions. Part of #3309
fwiw this could also be implemented as a dedicated SettingController to deal with the MatrixClient stuff (both reading and writing). It might not be worth the complexity though. |
After discussion with @nadonomy, we'll use the following dialog text for the prompt:
|
Ah yeah, good to know! It's probably okay for now as is I think, but if we were to sprinkle it around further, then I'd probably change to that. |
A further revision from Matthew:
|
This adds a prompt at the start of each session when the homeserver does not have any ICE servers configured. The fallback ICE server is only used if the user allows it. The dialog also recommends notifying the homeserver admin to rectify the issue. Fixes element-hq/element-web#10173
This stores the ICE server fallback permission in a device setting so it is remembered across sessions. Part of #3309
This moves the ICE fallback prompt out of session startup and instead it will now appear contextually when your either place a call with no ICE server from the homeserver or a call fails (in either direction). Fixes element-hq/element-web#10546
aa04607
to
adc3c69
Compare
When the homeserver does not have any ICE servers configured, this will cause a prompt to appear context of either placing a call or when a call fails. The user's choice is saved on the local device and can be changed in Settings. The fallback ICE server is only used if the user allows it. The dialog also recommends notifying the homeserver admin to rectify the issue.
Fixes element-hq/element-web#10173
Depends on matrix-org/matrix-js-sdk#1015