[Bug]: Screen sharing needs multiple confirmations

apollo13 commented 1 year ago

Before you file a bug report

[X] I have checked the issue tracker and have not found an issue that matches the one I'm filing.
[X] This issue is not a troubleshooting question. Troubleshooting questions go here: https://forum.mattermost.com/c/trouble-shoot/16.
[X] This issue is not a feature request. You can request features and make product suggestions here: https://mattermost.com/suggestions/.
[X] This issue doesn't reproduce on web browsers (such as in Chrome). If it does, issue reports go to the Mattermost Server repository.
[X] This issue reproduces on the most recent stable version, or the most recent prerelease version of the Mattermost Desktop App.
[X] I have read the contribution guidelines.

Mattermost Desktop Version

Version 5.5.0-rc.1 commit: f7b3a64

Operating System

Fedora 38

Mattermost Server Version

7.10.2

Steps to reproduce

Start a call
Click the screen sharing icon
Choose a screen/window via the portal and confirm (screenshot 1 below)
Get another confirmation dialog from mattermost for the shared window (screenshot 2 below)

Screenshot from 2023-09-02 13-11-43

Expected behavior

The second sharing screen (the white one from mattermost) should not show up. It doesn't show up when accessing the app in a browser directly, there it only shows the first dialog.

Observed behavior

After selecting the window to share, mattermost desktop shows yet another custom sharing dialog.

Log Output

Additional Information

No response

streamer45 commented 1 year ago

I assume this is the result of updating Electron. The screen permissions flow may need a review however this should only happen on Wayland setups which are likely not 100% supported.

apollo13 commented 1 year ago

Yes this is only on wayland and it at least is finally in a state where it is usable and only an UX issue. Since wayland is the default on at least Fedora and Ubuntu nowadays it would probably make sense to support it explicitly instead of focusing on X11 as primary target.

streamer45 commented 1 year ago

Agreed. When I am talking about support I really mean from the Electron side. There's only so much we can do on this one. Now that the update happened we can plan ways to make it feel a bit more native from the application.

apollo13 commented 1 year ago

From what I can gather the electron side of things is finally good! Before it crashed #2694 ;) But I just realized that I am in the wrong repo with this report as that might be fixable solely in the mattermost calls plugin.

streamer45 commented 1 year ago

That's good to know. I haven't looked in this yet but I think we'd need changes on this side as well because at the very least we need to understand what system we are on since this issue only exists on Wayland as far as I can tell.

devinbinnie commented 10 months ago

@streamer45 Any progress on this? I seem to recall Electron having a fix for this but I don't really remember.

streamer45 commented 10 months ago

I think we are still stuck at https://github.com/electron/electron/issues/37524 although I haven't really had a chance to test on Wayland still to see if there could be workarounds. Our internal ticket is https://mattermost.atlassian.net/browse/MM-53522

mattermost / desktop