Skip to content
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

[MM-54701] Fix development deep linking for login for Windows/Linux #2867

Merged
merged 3 commits into from
Oct 10, 2023

Conversation

devinbinnie
Copy link
Member

Summary

Windows and Linux were having issues logging into the app with the new auth changes in development mode. This is due to the way deep linking works on those OSes. The following fixes were made:

  • On Windows, we specify the exact binary and arguments when registering a deep link to ensure it opens the correct app and shouldn't interfere with other Electron apps.
  • On Linux, we drop a custom XDG desktop file and manually add protocol registration which should facilitate the correct protocol handling for the browser.

Ticket Link

https://mattermost.atlassian.net/browse/MM-54701

NONE

@devinbinnie devinbinnie added the 2: Dev Review Requires review by a core committer label Oct 3, 2023
Copy link
Member

@mvitale1989 mvitale1989 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Devin, LGTM!

I'd like to test it locally as well if possible, could you please tell me how this could be done?

@devinbinnie
Copy link
Member Author

Thanks Devin, LGTM!

I'd like to test it locally as well if possible, could you please tell me how this could be done?

You can just pull down the branch, run npm i and then npm run watch and it should just work :)

@devinbinnie devinbinnie added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Oct 5, 2023
@mvitale1989
Copy link
Member

Yip, can confirm that after following the development machine setup instructions (and in particular installing the required dependencies) the commands worked nicely 🎉

And the produced desktop file lgtm:

$ cat ~/.local/share/applications/mattermost-desktop-dev.desktop 
[Desktop Entry]
Name=Mattermost.Dev
Exec=[SNIP]/desktop/node_modules/electron/dist/electron [SNIP]/desktop/dist %U
Terminal=false
Type=Application
Icon=mattermost-desktop
StartupWMClass=Mattermost
Comment=Mattermost
MimeType=x-scheme-handler/mattermost-dev;
Categories=contrib/net;

@devinbinnie devinbinnie merged commit 02261c9 into mattermost:master Oct 10, 2023
11 checks passed
@amyblais amyblais added this to the v5.6.0 milestone Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants