search

Byron / open-rs

Open a path or URL with the system-defined program
http://byron.github.io/open-rs
MIT License
319 stars 49 forks source link

wasm32 support #52

Closed gilescope closed 1 year ago

gilescope commented 2 years ago

Does what it says on the tin. Opens a new tab with the given url. ( Fixes #49 )

Byron commented 2 years ago

Thanks a lot, much appreciated!

Could you share how I would test that? The answer to this question could lead to an update to the CI workflow to (possibly) allow testing this functionality, or to add documentation in the README, but at least I'd like to test it locally, somehow, but lack experience with the WASM workflow.

Thanks again for your help.

gilescope commented 2 years ago

Good question. I pull in bevy and trunk to test it but probably one of trunk’s examples can be a lot smaller. Let me have a look how to at least make it easy to manual test.

On Sun, 4 Sept 2022 at 08:04, Sebastian Thiel @.***> wrote:

Thanks a lot, much appreciated!

Could you share how I would test that? The answer to this question could lead to an update to the CI workflow to (possibly) allow testing this functionality, or to add documentation in the README, but at least I'd like to test it locally, somehow, but lack experience with the WASM workflow.

Thanks again for your help.

— Reply to this email directly, view it on GitHub https://github.com/Byron/open-rs/pull/52#issuecomment-1236274559, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGEJCCIZNTGKYR3L2NASGTV4RCWPANCNFSM6AAAAAAQD2MFXE . You are receiving this because you authored the thread.Message ID: @.***>

gilescope commented 1 year ago

While this works for FF and Chrome, on iOS and Safari this appears as a pop-up window and is blocked by default rather than being a new tab.

Byron commented 1 year ago

Thanks for the update. A large example would also be appreciated - maybe you can share the patch to apply to the bevy repository and the command to execute to build it and run it.

gilescope commented 1 year ago

There's a snag with the current implementation, it doesn't quite do what it says on the tin. Safari/iOS blocks popups by default so it seems like nothing happens. window.location.assign(url) will work but won't open in a separate tab. Maybe we check to see if 'Safari' is in the user agent and if so assign to the current window rather than open tab?

Byron commented 1 year ago

Maybe it's possible to give the user a choice and have JS decide if it should attempt a tab or change the URL of the current window instead. Hardcoding browser specifics seems like a well-intended step that can backfire as browsers change their rules or implementation.

Maybe it's possible to learn from other crates as well - there is opener for example which seems to be more popular and maybe does a few things right (while not having been updated for a year).

gilescope commented 1 year ago

Opening the link in the same window will work every time so maybe we should do that.

On Mon, 14 Nov 2022 at 13:26, Sebastian Thiel @.***> wrote:

Maybe it's possible to give the user a choice and have JS decide if it should attempt a tab or change the URL of the current window instead. Hardcoding browser specifics seems like a well-intended step that can backfire as browsers change their rules or implementation.

Maybe it's possible to learn from other crates as well - there is opener https://crates.io/crates/opener for example which seems to be more popular and maybe does a few things right (while not having been updated for a year).

— Reply to this email directly, view it on GitHub https://github.com/Byron/open-rs/pull/52#issuecomment-1313702083, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGEJCAT6BDSKWKLZBJDALTWII4XTANCNFSM6AAAAAAQD2MFXE . You are receiving this because you authored the thread.Message ID: @.***>

Byron commented 1 year ago

That's true, but in general I find this behaviour problematic as it's not semantically the same as what happens if open::that is used on any other platform: it's non-blockingly spawning a program that handles a given URL or file path. Closest to this is to open a new window or a new tab. If behaviour can reliably only be to open in the same tab or window, then for WASM I would expect a different function name to prevent people from cross-compiling without handling the semantic differences.

It looks like opener doesn't support WASM so here we are all alone with this.

What about providing a new function that indicates these semantic differences and provides the option to try opening in a new tab, explaining that this might not work depending on the browser you are in or the users extensions.

Byron commented 1 year ago

Closing due to inactivity. Please feel free to rebase against main and continue the conversation when there is time.