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

Add URL handler support for Linux desktops #56727

Merged
merged 1 commit into from Sep 13, 2018

Conversation

Projects
None yet
2 participants
@segevfiner
Contributor

segevfiner commented Aug 18, 2018

This works by adding a NoDisplay desktop file that registers for the MIME type x-scheme-handler/vscode to the deb and rpm package.

I added a URL Handler suffix to the name in that desktop file so it's discernible in menu editors from the main desktop file. It will appear like that in the UI in some places though.

image

See: Desktop Entry Specification - Recognized desktop entry keys, shared-mime-info-spec - URI scheme handlers

Fixes #48528

P.S. I wonder if desktop-file-install is really needed... I think the database is updated automatically when installing a desktop file from a deb.

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Sep 12, 2018

Member

Oh man, going down the rabbit hole here. While this is a good step, it's not enough:

This is still needed:

app.setAsDefaultProtocolClient(product.urlProtocol, process.execPath, ['--open-url', '--']);

But it always returns false on Linux. The problem is that electron is taking the default application name: https://github.com/electron/electron/blob/6f91af93433df4e9c0e7fb592e5b2dcb0b1c7888/atom/browser/browser_linux.cc#L55

Which resolves to the CHROME_DESKTOP variable: https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/ui/libgtkui/gtk_util.cc#133

Which is set on the Electron app: https://github.com/electron/electron/blob/f8828aa804508e4987db6fe249c55b1a51add969/atom/browser/api/atom_api_app.cc#L862

And this is the wrong .desktop file name. We need to put in the good one. Luckily, this can be overridden in the package.json: https://github.com/electron/electron/blob/6f91af93433df4e9c0e7fb592e5b2dcb0b1c7888/lib/browser/init.js#L138

Wow. Electron needs some documentation here, that took too long to figure out. I'll pick this PR up, clean it, get these findings in there and merge it tomorrow.

Member

joaomoreno commented Sep 12, 2018

Oh man, going down the rabbit hole here. While this is a good step, it's not enough:

This is still needed:

app.setAsDefaultProtocolClient(product.urlProtocol, process.execPath, ['--open-url', '--']);

But it always returns false on Linux. The problem is that electron is taking the default application name: https://github.com/electron/electron/blob/6f91af93433df4e9c0e7fb592e5b2dcb0b1c7888/atom/browser/browser_linux.cc#L55

Which resolves to the CHROME_DESKTOP variable: https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/ui/libgtkui/gtk_util.cc#133

Which is set on the Electron app: https://github.com/electron/electron/blob/f8828aa804508e4987db6fe249c55b1a51add969/atom/browser/api/atom_api_app.cc#L862

And this is the wrong .desktop file name. We need to put in the good one. Luckily, this can be overridden in the package.json: https://github.com/electron/electron/blob/6f91af93433df4e9c0e7fb592e5b2dcb0b1c7888/lib/browser/init.js#L138

Wow. Electron needs some documentation here, that took too long to figure out. I'll pick this PR up, clean it, get these findings in there and merge it tomorrow.

@segevfiner

This comment has been minimized.

Show comment
Hide comment
@segevfiner

segevfiner Sep 12, 2018

Contributor

We need to go deeper

I think setAsDefaultProtocolClient is using a different way to register a "protocol client"/"scheme handler". You can see that electron/atom/browser/browser_linux.cc:45-60 uses xdg-settings. Meaning that if you get it to work, it should make this PR, which uses a desktop file set to hidden, redundant.

The behavior of setAsDefaultProtocolClient on Linux, and it's interaction with the desktop name thing, seems to be missing from the electron docs.

Contributor

segevfiner commented Sep 12, 2018

We need to go deeper

I think setAsDefaultProtocolClient is using a different way to register a "protocol client"/"scheme handler". You can see that electron/atom/browser/browser_linux.cc:45-60 uses xdg-settings. Meaning that if you get it to work, it should make this PR, which uses a desktop file set to hidden, redundant.

The behavior of setAsDefaultProtocolClient on Linux, and it's interaction with the desktop name thing, seems to be missing from the electron docs.

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Sep 13, 2018

Member

We still need a valid *.desktop file which is able to open uris, since that's what xdg-settings needs. The default code.desktop file can't really handle opening uris. I'll see whether we make it handle uris or whether we ship this other .desktop file.

Yeah, absolutely, once I'm done I'll file an issue to improve the docs there.

Member

joaomoreno commented Sep 13, 2018

We still need a valid *.desktop file which is able to open uris, since that's what xdg-settings needs. The default code.desktop file can't really handle opening uris. I'll see whether we make it handle uris or whether we ship this other .desktop file.

Yeah, absolutely, once I'm done I'll file an issue to improve the docs there.

@segevfiner

This comment has been minimized.

Show comment
Hide comment
@segevfiner

segevfiner Sep 13, 2018

Contributor

Hmm so xdg-settings requires a desktop file that can handle urls... Just placing the desktop file and running update-desktop-database was actually enough to get the vscode scheme to work. It's possible all xdg-settings does is change the default when there are multiple applications that can handle the same scheme.

It's one main command line that can be associated with MIME types per desktop file, as far as I know. What the desktop name variable in electron should point might also be an issue, since I'm not sure what more it might be used for besides setAsDefaultProtocolClient.

Contributor

segevfiner commented Sep 13, 2018

Hmm so xdg-settings requires a desktop file that can handle urls... Just placing the desktop file and running update-desktop-database was actually enough to get the vscode scheme to work. It's possible all xdg-settings does is change the default when there are multiple applications that can handle the same scheme.

It's one main command line that can be associated with MIME types per desktop file, as far as I know. What the desktop name variable in electron should point might also be an issue, since I'm not sure what more it might be used for besides setAsDefaultProtocolClient.

@joaomoreno joaomoreno merged commit 8f83890 into Microsoft:master Sep 13, 2018

2 checks passed

VSTS: VS Code 20180818.3 succeeded
Details
license/cla All CLA requirements met.
Details
@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Sep 13, 2018

Member

As expected, we still needed a secondary hidden .desktop file, so I just took your changes. All that was left was to set desktopName in package.json: bb23217#diff-8b4de73d116d9f17bd7c5384ecee6ee2R245 After doing that, I triggered a build and verified that it works!

Thanks for this push, great job! 🍻

Do you mind testing it in tomorrow's release? A good URL example is: vscode-insiders:extension/jolaleye.horizon-theme-vscode. It should open that extension in Code.

Member

joaomoreno commented Sep 13, 2018

As expected, we still needed a secondary hidden .desktop file, so I just took your changes. All that was left was to set desktopName in package.json: bb23217#diff-8b4de73d116d9f17bd7c5384ecee6ee2R245 After doing that, I triggered a build and verified that it works!

Thanks for this push, great job! 🍻

Do you mind testing it in tomorrow's release? A good URL example is: vscode-insiders:extension/jolaleye.horizon-theme-vscode. It should open that extension in Code.

@joaomoreno joaomoreno referenced this pull request Sep 24, 2018

Closed

Test: Linux URL handler support #59252

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment