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

Package to open browser url from gio app #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

g45t345rt
Copy link

@g45t345rt g45t345rt commented Aug 21, 2023

Follow up on this pull request gioui/gio#119

Simple usage

import "gioui.org/x/browser"
browser.OpenUrl("https://gioui.org/")

Cross-platform implemented

Android - Intent object
Windows - exec command "cmd /c start"
Unix - exec command "xdg-open"
Macos - exec command "open"
IOS - UIApplication openURL
JS - window.open

P.S I'm coding blind for IOS implementation. I don't have a mac + I have no idea how to test/build either.

@gedw99
Copy link

gedw99 commented Aug 21, 2023

@g45t345rt
I have Mac and iOS and can try it out.

It’s a bummer that this repo does not have example sub folders . The convention is to have the examples in the gio-examples repo

@whereswaldon would it be a terrible idea to allow examples to go into this actual repo ?

@whereswaldon
Copy link
Member

@whereswaldon would it be a terrible idea to allow examples to go into this actual repo ?

I keep them separate out of a desire to keep the dependency tree for gio-x as lean as possible. gio-example has a huge dependency tree because some examples import a bespoke dependency to demonstrate something cool with it. I recognize that it would be useful to have some examples in the repo, but it would also add a fair bit of complexity. I'd prefer to keep things as they are for now.

Copy link
Member

@whereswaldon whereswaldon left a comment

Choose a reason for hiding this comment

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

Thank you very much for the PR. I will try to review it within the next week. Please ping me if you don't hear anything.

@inkeliz
Copy link
Contributor

inkeliz commented Aug 21, 2023

@g45t345rt
Copy link
Author

@inkeliz Damn, I didn't know this existed :O Why is this not part of gio-x package?

@whereswaldon
Copy link
Member

@inkeliz Damn, I didn't know this existed :O Why is this not part of gio-x package?

We've talked about merging it, but I don't remember why we never did.

@g45t345rt
Copy link
Author

Unrelated to this topic, I started working on camera access. You can find the code in this branch:
https://github.com/g45t345rt/gio-x/tree/camera/camera
It's a simple camera preview draw to paint.ImageOp, and we have potential to integrate more features from the Camera API, but for now, it could serve as a foundation starting point. I still need to implement IOS.
Anyway, let me know what you think.

@inkeliz, pls tell me you haven't done this already :S I've searched your repos and could'nt find anything.

@inkeliz
Copy link
Contributor

inkeliz commented Aug 25, 2023

@inkeliz, pls tell me you haven't done this already :S I've searched your repos and could'nt find anything.

I didn't. Most of packages are available under https://github.com/gioui-plugins/gio-plugins. Here you can see what I have consider to implement (https://github.com/orgs/gioui-plugins/projects/1).

I had consider to implement Camera, Microphone, Geolocation/GPS in the past. However, that is quite hard to do it right without a proper use-case.


I'm not sure how the camera is suppose to work (if that opens the camera app; or if that "embeds" the frame inside Gio). If is the second method (which receives an image-per-frame): I think it might be a good ideia to expose some "ring-buffer". Instead of allocate bytes each frame, you can re-use the same byte-array (kind of triple-buffer). That also prevents copying from Java to Go again, which may improve performance on low-low-low-end devices.

Copy link
Member

@whereswaldon whereswaldon left a comment

Choose a reason for hiding this comment

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

This is overwhelmingly similar to giohyperlink, to the point that I wonder whether @inkeliz would be willing to combine your efforts and get the result landed in gioui.org/x. In particular, you've found a way to implement support for Android that does not require adding an extra handler to the window event loop, which is a nice simplification.

I've been diffing this code against giohyperlink as I reviewed it to see the various strategies for implementing things, and I think something along the lines of giohyperlink's scheme validation is probably necessary. Opening URLs can do practically anything if they might trigger a custom application-defined URL scheme like zoom:// or slack://. Frequently applications will need to open content provided by users, and they need to guard against malicious attempts to manipulate URL handling.

As such, I think we really need the union of this PR and giohyperlink to provide the safest, simplest path forward. In particular, I think the goal should be:

  • no event loop modifications needed for basic use (it seems like it may only be necessary for some fallback logic in js)
  • use parsed, structured URLs instead of strings, and offer control over which URL schemes are able to be opened
  • use the simplest integration option available on each platform

Comment on lines +10 to +13
func OpenUrl(url string) error {
js.Global().Call("open", url, "_blank", "")
return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

How widely did you test this across browser platforms? I'm curious whether @inkeliz's sophisticated hacks are needed here.

Copy link
Contributor

Choose a reason for hiding this comment

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

In Safari, the open function must be invoked inside onClick event. However, Gio gets and releases this event.

If open is called without direct user interaction, outside of onClick, it may not always work. It's inconsistent, so sometimes it works, but often it doesn't.

The "hack" requires the user to click anywhere in the page to open, it's not perfect, but works as fallback.

)

func OpenUrl(url string) error {
cmd := exec.Command("cmd", "/c", "start", url)
Copy link
Member

Choose a reason for hiding this comment

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

Given that we can avoid spawning a cmd instance for this, I think I'd prefer using that more direct approach.

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

Successfully merging this pull request may close these issues.

4 participants