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

open doesn't work on some user machines, why not use electron.shell.openPath? #108

Closed
Tracked by #102
MichaelBelousov opened this issue Feb 7, 2023 · 8 comments · Fixed by #120
Closed
Tracked by #102
Assignees
Labels
bug Something isn't working electron Related to the desktop/electron authorization

Comments

@MichaelBelousov
Copy link
Contributor

Can we switch from using open here
to electron.shell.openPath, as recommended in the README of open itself?

I have found users where open doesn't work to open the browser (as it's used here anyway), where as electron's shell.openPath works fine.

@MichaelBelousov MichaelBelousov self-assigned this Feb 8, 2023
@MichaelBelousov MichaelBelousov added the bug Something isn't working label Feb 8, 2023
@calebmshafer calebmshafer added the electron Related to the desktop/electron authorization label Feb 25, 2023
@ben-polinsky ben-polinsky self-assigned this Mar 6, 2023
@ben-polinsky
Copy link
Collaborator

ben-polinsky commented Mar 23, 2023

Reopening as openPath is incorrect - that is for paths not urls and fails on mac os. openExternal is the way to go. I imagine Windows is able to forward the url from explorer to default browser.

@MichaelBelousov
Copy link
Contributor Author

MichaelBelousov commented Apr 10, 2023

The current latest version of the desktop auth client doesn't work on linux and mac. Can reproduce on a desktop application I'm working on which is using the latest (@itwin/electron-authorization@0.13.0)

I can confirm that openExternal works on linux, but I haven't tried windows, going to soon.

@ben-polinsky
Copy link
Collaborator

Sorry @MichaelBelousov it's gotten caught up in a PR with some other things. I'll extract that since it is an easy fix and we can release it asap.

@MichaelBelousov
Copy link
Contributor Author

I wouldn't describe it as urgent on my end @ben-polinsky, just been a bit and wanted to make sure it's still being worked on. Can wait for the larger PR too.

@ben-polinsky
Copy link
Collaborator

Definitely a priority.

@MichaelBelousov
Copy link
Contributor Author

A colleague confirmed using openExternal works on windows.

@ben-polinsky
Copy link
Collaborator

merged.. waiting for release

@ben-polinsky
Copy link
Collaborator

ben-polinsky commented May 17, 2023

This is released in @itwin/electron-authorization@0.14.0
https://github.com/iTwin/auth-clients/releases/tag/%40itwin%2Felectron-authorization_v0.14.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working electron Related to the desktop/electron authorization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants