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

[Bug]: MimeType missing in Desktop Entry File on generic linux #2942

Open
6 tasks done
Lotbert opened this issue Jan 8, 2024 · 11 comments
Open
6 tasks done

[Bug]: MimeType missing in Desktop Entry File on generic linux #2942

Lotbert opened this issue Jan 8, 2024 · 11 comments
Labels

Comments

@Lotbert
Copy link

Lotbert commented Jan 8, 2024

Checks before filing an issue

Mattermost Desktop Version

5.6.0

Operating System

NixOS 23.05

Mattermost Server Version

No response

Steps to reproduce

  1. Login using Gitlab SSO.
  2. Authorize Mattermost in Gitlab
  3. Allow the Browser to open in...

Expected behavior

The Browser asks to be allowed to open Mattermost. We get redirected to the Mattermost Desktop App with a logged-in user.

Observed behavior

The Browser asks us to allow it to open xdg-open (instead of Mattermost). After we allow it, the default Browser is opened with the following url in the address bar:
mattermost-dev://mattermost.example.de/login/desktop?client_token=dev-<client_token>&isDesktopDev=true&server_token=<server_token>

Log Output

I forgot to save the log outputs.

Additional Information

Two things are still wrong in the current master while writing this. I'll explain what, using two patches that fixed my local installation.

  1. The binaries packaged in the released tar archive use mattermost-dev:// as prefix for the mattermost desktop app, a possible fix could be to build the mattermost-desktop app with the ELECTRON_IS_DEV environment variable set to 0 to get rid of the -dev (I also explain a workaround using another MimeType in the code explained below)
modified   package.json
@@ -59,7 +59,7 @@
     "package:mac-with-universal": "npm-run-all build-prod && electron-builder --mac --x64 --arm64 --universal --publish=never",
     "package:mas": "cross-env NODE_ENV=production IS_MAC_APP_STORE=true npm-run-all check-build-config build && electron-builder --mac mas --universal --publish=never",
     "package:mas-dev": "cross-env NODE_ENV=production IS_MAC_APP_STORE=true npm-run-all check-build-config build && electron-builder --mac mas-dev --universal --publish=never",
-    "package:linux": "npm-run-all package:linux-all package:linux-appImage",
+    "package:linux": "set ELECTRON_IS_DEV=0 && npm-run-all package:linux-all package:linux-appImage",
     "package:linux-appImage": "npm-run-all build-prod-upgrade package:linux-appImage-x64 package:linux-appImage-arm64",
     "package:linux-appImage-x64": "electron-builder --linux tar.gz appimage --x64 --publish=never",
     "package:linux-appImage-arm64": "cross-env CC=aarch64-linux-gnu-gcc CXX=aarch64-linux-gnu-g++ electron-builder --linux tar.gz appimage --arm64 --publish=never",
  1. The Desktop Entry Exec Attribute does not need double quotes surrounding the mattermost-desktop path. See: https://wiki.archlinux.org/title/desktop_entries#Modify_command_line_arguments
modified   src/assets/linux/create_desktop_file.sh
@@ -9,7 +9,7 @@ cat <<EOS > Mattermost.desktop
 [Desktop Entry]
 Name=Mattermost
 Comment=Mattermost Desktop application for Linux
-Exec="${FULL_PATH}/mattermost-desktop" %U
+Exec=${FULL_PATH}/mattermost-desktop %U
 Terminal=false
 Type=Application
 # I am using `MimeType=x-scheme-handler/mattermost-dev` as a workaround. 
 # The following line is still missing in v5.6.0
 MimeType=x-scheme-handler/mattermost

Also beware of the last three lines in the code above.

@lesteve
Copy link

lesteve commented Feb 13, 2024

This seems to affect the ArchLinux mattermost-desktop 5.6.0 package as well.

If I looked in my Firefox console I see:

Prevented navigation to “mattermost-dev://mattermost.inria.fr/login/desktop?client_token=<dev-...>&isDesktopDev=true&server_token=<...>” due to an unknown protocol.

@christianhueserhzdr
Copy link

christianhueserhzdr commented Feb 26, 2024

We are having the same issues with the current Mattermost Desktop App in v5.6.0 with the same console message "Prevented navigation to 'mattermost://...' due to an unknown protocol". It is failing in Firefox and working in Chromium on Debian.

@devinbinnie
Copy link
Member

So it looks like the Arch Linux build for some reason is being built in development mode, despite the fact that they are setting NODE_ENV=production. The mattermost-dev: protocol should only be necessary if you're developing the app so all official releases should be using the mattermost: protocol, which is baked into the .desktop file.

I tried to reproduce their build process but I wasn't able to reproduce the issue, looks like it builds in prod mode for me.

Sounds like this is what's causing this issue for most people, if you're seeing mattermost-dev: that's likely the app misreporting that it's in dev mode.

@Lotbert
Copy link
Author

Lotbert commented Feb 28, 2024

@devinbinnie
Correct, it is build in dev mode for some reason.
Did you use the package:linux script from the package.json to build the app?

@devinbinnie
Copy link
Member

@devinbinnie Correct, it is build in dev mode for some reason. Did you use the package:linux script from the package.json to build the app?

@Lotbert I'm running build and then package:linux-all-x64 just as the Arch script does. Did not build in dev mode for me.

@Lotbert
Copy link
Author

Lotbert commented Feb 28, 2024

Ok. I think somehow the isDev in the following line:

const protocol = isDev ? 'mattermost-dev' : mainProtocol;

evaluates to true. But why?

Digging a bit deeper i had a look into electron-is-dev and found the following line

const isDev = isEnvSet ? getFromEnv : !electron.app.isPackaged;

Since i didn't find a file containing ELECTRON_IS_DEV maybe electron.app.isPackaged is a falsy value in the environment executing the app that produces the mentioned bug(s).

I just wanted to leave this here...

@Lotbert
Copy link
Author

Lotbert commented Feb 28, 2024

@devinbinnie isn't the ci pipeline executing package:linux here:
https://github.com/mattermost/desktop/blob/v5.6.0/.github/workflows/release.yaml#L58

Maybe i am getting it wrong...

@devinbinnie
Copy link
Member

@devinbinnie isn't the ci pipeline executing package:linux here: https://github.com/mattermost/desktop/blob/v5.6.0/.github/workflows/release.yaml#L58

Maybe i am getting it wrong...

Yes the pipeline is, which basically executes the same thing:
package:linux will run build-prod and package:linux-all-x64
build-prod just runs NODE_ENV=production npm run build

To the above point it's possible there's a bug in that package. Grabbing the .tar.gz file from the GitHub release, it's running in Production Mode as well. The only place I can see this reproduced is from the Arch package from pacman.

@enticedwanderer
Copy link

The reason for this I think is the behavior of electron.app.isPackaged. This post summarizes the behavior. But the gist is that at least for the Arch linux, the package for electron names the executable as electron27 while the isPackaged flag behavior explicitly expects it to be exactly electron (see arch linux package). Since it isn't it assumes it isn't packaged and thus it behaves like it is in dev mode.

Likely when you build it yourself you end up running it against your normal electron package with matching binary name. I'm not sure what's the best way to go about fixing this, but the easiest seems to be to just set ELECTRON_FORCE_IS_PACKAGED env in the build script.

@Lotbert
Copy link
Author

Lotbert commented Mar 1, 2024

IMHO using set ELECTRON_IS_DEV=0 should also work and has clearer semantics if one only looks at the name of the env var. That's also what I proposed to try in the first of the patches I posted originally.

@Lotbert
Copy link
Author

Lotbert commented Apr 8, 2024

IMHO using set ELECTRON_IS_DEV=0 should also work and has clearer semantics if one only looks at the name of the env var. That's also what I proposed to try in the first of the patches I posted originally.

This works @lzszt and me just did that in a PR for the respective nixpkg and it fixed the bug.

See NixOS/nixpkgs#279645 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants