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

io,app: add deeplinking support #117

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

inkeliz
Copy link
Contributor

@inkeliz inkeliz commented Jul 8, 2023

Now, it's possible to launch one Gio app using a custom URI scheme, such as gio://some/data.

This feature is supported on Android, iOS, macOS and Windows, issuing a new deeplink.Event,
containing the URL launched. If the program is already opened, one deeplink.Event will be
sent to the current opened app.

Limitations:
On Windows, if the program uses deeplink (compiled with -deeplink), then just a single
instance of the app can be open. In other words, just a single myprogram.exe can
be active.

Security:
Deeplinking have the same level of security of clipboard. Any other software can send such
information and read the content, without any restriction. That should not be used to transfer
sensible data, and can't be fully trusted.

Setup/Compiling:
In order to set the custom scheme, you need to use the new -deeplink flag in gogio, using
as -deeplink gio will listen to gio://.

If you are not using gogio you need to defined some values, which varies for each OS:

macOS/iOS - You need to define the following Properly List:

<key>CFBundleURLTypes</key>
<array>
  <dict>
	<key>CFBundleURLSchemes</key>
	<array>
	  <string>yourCustomScheme</string>
	</array>
  </dict>
</array>

Windows - You need to compiling using -X argument:

-ldflags="-X "gioui.org/app.schemesDeeplink=yourCustomScheme" -H=windowsgui"

Android - You need to add IntentFilter in GioActivity:

<intent-filter>
	<action android:name="android.intent.action.VIEW"></action>
	<category android:name="android.intent.category.DEFAULT"></category>
	<category android:name="android.intent.category.BROWSABLE"></category>
	<data android:scheme="yourCustomScheme"></data>
</intent-filter>

That assumes that you still using GioActivity and GioAppDelegate, otherwise more
changes are required.

@inkeliz
Copy link
Contributor Author

inkeliz commented Jul 11, 2023

I use this code for testing: https://gist.github.com/inkeliz/11c071ec4bf8f922a3cb48fd0cbb3b95.

That is based on gio-plugins/deeplink/demo, but modified, since the events comes from window.Event(). You need to use gioui/gio-cmd#9 to compile, and need to set -deeplink yourScheme in gogio, to listen for yourScheme://.

@inkeliz
Copy link
Contributor Author

inkeliz commented Jul 11, 2023

@gedw99 That panic was from gio-plugins, not from gio (this current PR). You should not use gio-plugins/deeplink anymore, since everything was ported to Gio (this patch). I didn't use panic anywhere here (https://github.com/gioui/gio/pull/117/files).

I park the code here, https://github.com/inkeliz/deeplink-demo. It uses the replace to point to my fork/pr.

git clone [email protected]:inkeliz/deeplink-demo.git
cd deeplink-demo 
go get gioui.org/cmd
go run gioui.org/cmd/gogio -target macos -deeplink test -o demo.app .
open demo_arm64.app

That will listen to test://. Then, you can open test://its/works to test. On MacOS, I think it requires MacOS 10.8 or 10.14, I don't remember.

@inkeliz
Copy link
Contributor Author

inkeliz commented Jul 19, 2023

Just to remind me in the future:

  • Domain names with accents (ã, ç, ù or emojis) have different behaviours. On Windows it's escapes similar to URL paths. On macOS/iOS it uses PunnyCode.

    • Document that behavior or try to match it (converting domain-accents to punnycode).
    • Maybe add tests for that?
  • Maximum URL size can vary for each OS. On iOS/macOS it can use upto 2GB, on Windows seems like 2048 characters, and CMD params have a limit of ~8K chars.

    • Add some documentation, assuming that anything < 2048 is safe to work.
    • Maybe, artificially enforce that limitation everywhere? Don't make sense, but makes easier to test.
  • Improve some documentation, on io/transfer (currently, io/deeplink). Telling that -schemes (currently, -deeplink) must be defined in gogio. Currently, Gio have one package and Event, but no clue about how to use that.

@mearaj
Copy link
Contributor

mearaj commented Jul 20, 2023

Hi @inkeliz, thanks a lot for implementing this feature.

Are there any chances of supporting Linux?

@inkeliz
Copy link
Contributor Author

inkeliz commented Jul 21, 2023

Hi @inkeliz, thanks a lot for implementing this feature.

Are there any chances of supporting Linux?

@mearaj, I don't think it's difficult to implement. However, personally, I don't use Linux, either my target-audience (it's < 1% that uses Linux).

But, seems that you need to create a file, say ~/.local/share/applications:

[Desktop Entry]
Name=MyApp
Exec=/path/to/your/program %U
Type=Application
StartupNotify=true
MimeType=x-scheme-handler/yourscheme

Then run update-desktop-database (https://www.unix.com/man-page/centos/1/update-desktop-database/).

However, its works similar to Windows: that will open the app passing the URL as the first parameter. So, it will require to create some IPC to send the URL to the active window, and make it a single-instance (either by pipe or d-bus).

Currently, gogio don't support Linux. So, I'm not sure how will set those schemes. Since I have 0 experience with desktop-linux, I'm not going to work on that. At least, not now. :S

@mearaj
Copy link
Contributor

mearaj commented Jul 21, 2023

Hi @inkeliz, thanks a lot for implementing this feature.
Are there any chances of supporting Linux?

@mearaj, I don't think it's difficult to implement. However, personally, I don't use Linux, either my target-audience (it's < 1% that uses Linux).

But, seems that you need to create a file, say ~/.local/share/applications:

[Desktop Entry]
Name=MyApp
Exec=/path/to/your/program %U
Type=Application
StartupNotify=true
MimeType=x-scheme-handler/yourscheme

Then run update-desktop-database (https://www.unix.com/man-page/centos/1/update-desktop-database/).

However, its works similar to Windows: that will open the app passing the URL as the first parameter. So, it will require to create some IPC to send the URL to the active window, and make it a single-instance (either by pipe or d-bus).

Currently, gogio don't support Linux. So, I'm not sure how will set those schemes. Since I have 0 experience with desktop-linux, I'm not going to work on that. At least, not now. :S

No worries, I understand. Thank you so much for the reply and providing the helpful info. :)

@mearaj
Copy link
Contributor

mearaj commented Jul 21, 2023

@mearaj

The groundwork laid down by the changes in gogio cmd by @inkeliz will mean its relatively easy to add linux support if you want to have a crack at it. I dont run linux and so not easy for me to do it and test it works.

HI @gedw99

Actually I am able to figure it out on my Arch Linux, the approach is the same as suggested by @inkeliz , but the issue is that it creates a separate instance of my app. I am trying to figure out the optimal solution. Once I find the best solution that works on most linux flavors(if not all), then I will most probably implement this feature in gio.
Thanks :)

@mearaj
Copy link
Contributor

mearaj commented Jul 22, 2023

Hi @ALL,
I was able to implement this feature on my platform (Arch Linux) using @inkeliz suggested approach and unix socket. Need your help in how shall I proceed? It will take atleast 2-3 days for me to implement it in gogio and also this is my first time working with gogio specifically. Shall I wait for this commit to be merged or shall I create a separate PR against inkeliz:add-deeplink or whatever you say.

@mearaj
Copy link
Contributor

mearaj commented Jul 22, 2023

Hi @ALL, I was able to implement this feature on my platform (Arch Linux) using @inkeliz suggested approach and unix socket. Need your help in how shall I proceed? It will take atleast 2-3 days for me to implement it in gogio and also this is my first time working with gogio specifically. Shall I wait for this commit to be merged or shall I create a separate PR against inkeliz:add-deeplink or whatever you say.

I saw the gogio code, it doesn't supports linux as pointed out by @inkeliz. I have decided to create a separate repo for this feature on linux. It will support both gio and non gio apps. Will post the link here. Thanks :)

@inkeliz
Copy link
Contributor Author

inkeliz commented Jul 22, 2023

Another alternative is to create some app.Scheme("yourscheme") option. Problems: that only works with Linux/Windows, because macOS/iOS/Android requires some manifest, and can't register it on runtime. So, what happens on iOS/Android?!

Notice: My suggestion is NOT an app.NewWindow option. Because app.Scheme applies to instances, and not per-window. So, configuring it on NewWindow can be impossible and/or too late. It could be app.Scheme or app.NewInstance(app.Scheme("yourscheme")).

My idea with gogio is to enforce the "compile-time", even when not required (Windows, and Linux). Since Gio usually tries to hide OS-specific APIs. If most OSes requires "compile-time" stuff (manifest...), then let's enforce it.

Another approach, is to read Golang's AST. You use app.Scheme, and only that. Then, gogio will search app.Scheme usage (i.e app.Scheme("abc")) then gogio will compile with abc in Manifest. It's not perfect (conditions and so on), but that also opens gogio to alot of "hidden-magic", so it's seems great and terrible. I think @eliasnaur will not like that too.


The easiest alternative: create a private-variable and then use ldflags to define it. That already happens:

gio/app/app.go

Line 27 in d62057a

// go build -ldflags="-X 'gioui.org/app.ID=org.gioui.example.Kitchen'" .

In this patch, a new one:

gio/app/os_windows.go

Lines 1016 to 1018 in fe42c61

// schemesDeeplink is a list of schemes, comma separated, that most be
// defined using -X compiler ldflag.
var schemesDeeplink string

It not requires gogio, but still required to set at compile-time (similar to Windows). I don't know how difficult is to port gogio to Linux. It just create a single executable with some icon, and use the ldflags to set the variable. :S

@mearaj
Copy link
Contributor

mearaj commented Jul 22, 2023

Hi @inkeliz , I mostly understood what you meant. I will try to implement it in gio itself by looking at the way you did for Windows. In case if I can't do it, I will share my implementation repo link here and will discuss with all of you guys :)

@mearaj
Copy link
Contributor

mearaj commented Jul 24, 2023

Hi @inkeliz,@gedw99 and everyone. Desktop Entry Specs

I could successfully do it on gio without gogio, but need your help further, also facing some issues currently.
Currently flow is as follows (Only for unix/linux)

Before Window Creation:

  • App checks if another instance of our app is running by trying to dial a connection using unix socket and expects two type of errors.
    • Error One: It says that file doesn't exists, in which case we assume that no other instance of our app is running.
      We install the app on user's platform. Start listening to socket connection and whenever any valid url data is received
      from the socket, we fire window.Event(transfer.URLEvent{URL: urlPtr})
    • Error Two: It says connection refuse, which implies that only socket file exists but another instance of our app is not
      running because it never refuses connection. Here we delete that file and continue with the above step.(Error One)
    • AnyOtherError: We exit the app. (This has no yet happened)
    • Err is nil: Here we are certain that the another instance of our app is running. We pass the arguments to that app through
      unix socket and exit the app with error "another instance of this app is already running"

The issue currently I am facing is that I don't know how to proceed further. In the current state whenever app receives any uri, it fires window.Event(transfer.URLEvent{URL: urlPtr}).
Another issue is that on unix it's possible to pass multiple uri's. In the app.desktop entry file, in Exec, there's %U for multiple uri's and %u for single uri. Is it possible to change the signature of type URLEvent struct {URL *url.URL} to URLEvent struct {URLs []*url.URL}, this way on unix we would be able to share multiple uris in one request?

I have created a PR at your branch.(It's not a final PR, but just for your review)
Please feel free to make any changes and also please do suggest me how shall I proceed further?
Thanks :)

mearaj added a commit to mearaj/gio that referenced this pull request Jul 24, 2023
discussion at gioui#117 (comment)

Signed-off-by: Mearaj Bhagad <[email protected]>
@inkeliz
Copy link
Contributor Author

inkeliz commented Jul 25, 2023

Another issue is that on unix it's possible to pass multiple uri's. In the app.desktop entry file, in Exec, there's %U for multiple uri's and %u for single uri. Is it possible to change the signature of type URLEvent struct {URL *url.URL} to URLEvent struct {URLs []*url.URL}, this way on unix we would be able to share multiple uris in one request?

On macOS (or iOS) it also receives multiple URLs, that is translated to multiple transfer.URLEvent (one URLEvent per URL). Do you have any use-case for multiple "bundled/combined" URLs?

@mearaj
Copy link
Contributor

mearaj commented Jul 25, 2023

Another issue is that on unix it's possible to pass multiple uri's. In the app.desktop entry file, in Exec, there's %U for multiple uri's and %u for single uri. Is it possible to change the signature of type URLEvent struct {URL *url.URL} to URLEvent struct {URLs []*url.URL}, this way on unix we would be able to share multiple uris in one request?

On macOS (or iOS) it also receives multiple URLs, that is translated to multiple transfer.URLEvent (one URLEvent per URL). Do you have any use-case for multiple "bundled/combined" URLs?

Thanks for asking.
There are multiple reasons,

  • Android's intent like behavior on Unix/Linux apps, where we can send and receive multiple uris.
  • I am creating a firebase based frontend app where i need to send user credentials and other info as multiple uris to the same app on desktop. It will be very useful for communication between desktop and browser.
  • On Unix/Linux, in the current implementation the app can receives uri directly from the socket or as arguments where each argument is a separate URI. Allowing multiple URI will give it a uniform way to tackle those.
  • Most important reason is that it's more flexible as both options remains open. If app developer is interested in only single uri then he can just select the first element from the slice. If he is interested in multiple uris, then he can do that as well.

These are the reasons I can currently think of :)

@inkeliz
Copy link
Contributor Author

inkeliz commented Jul 26, 2023

I am creating a firebase based frontend app where i need to send user credentials and other info as multiple uris to the same app on desktop. It will be very useful for communication between desktop and browser.

Can you provide an example of how to open multiple URLs? Honestly, I've tried testing this using window.Open and <a href> in a browser, as well as testing with the browser/explorer address bar. In both cases, I'm able to open a single URL. I'm unsure how to open a list of URLs. Of course, I can open multiple URLs, but each one is independent from the others (multiple transfer.URLEvent). If you have while(true) { window.Open("yourscheme://data")} each call is a new URLEvent, since it only opens one URL per time.

On Android, getData returns a single URL (https://developer.android.com/reference/android/content/Intent#getData()), and the same is true for iOS and Windows.

Currently, only macOS (and Linux) provide a list of URLs, and I'm not sure how to test that.


Changing to []*url.URL is easy, but it can get tricky figuring out when they'll "bundle" or not. Since they will not always be "bundled", you need to treat both cases in the same way. In the end: You need to handle multiple transfer.URLEvent items and a single "combined" transfer.URLEvent.

@mearaj
Copy link
Contributor

mearaj commented Jul 26, 2023

Ahh yes, you are right. On linux I can convert multiple url into a single url . This way the app will have a common interface, so yeah your reason sounds very reasonable. So let's keep the way it is currently. I will make some changes in the current linux implementation and get back to you and will ask for your help again. Thanks a lot @inkeliz.

@mearaj
Copy link
Contributor

mearaj commented Jul 26, 2023

Hi @inkeliz , I have pushed changes to PR.

@inkeliz inkeliz requested a review from eliasnaur July 29, 2023 19:55
app/window.go Outdated
Comment on lines 899 to 900
case transfer.URLEvent:
w.out <- e2
Copy link
Contributor

Choose a reason for hiding this comment

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

This is simple, but global. I believe transfer.URLEvents should be routed like transfer.DataEvent. No?

Copy link
Contributor Author

@inkeliz inkeliz Aug 4, 2023

Choose a reason for hiding this comment

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

I'm not sure. In the case of DataEvent it can target a specific area of the UI, right? Like, drag/drop at specific zone. In the case of URLEvent that is somewhat external. At least in my case, it will require to have SomeOp{Tag: tag} in the main. However, that already happens with the "Back Button" (which is now on InputOp).

I don't know if that should be routed, and if it should be a new Op or use TargetOp? The MIME field is strange in that case. Is MIME the scheme? What happens if want the listen "all schemes"?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think (most/all) events should be routed, for composability of components. Without routing, you can't just use some widget and forget about its internals. With routing, you don't need to know that the widget listens for a particular dnd or file open.

I don't know if that should be routed, and if it should be a new Op or use TargetOp? The MIME field is strange in that case. Is MIME the scheme? What happens if want the listen "all schemes"?

This is a problem for dnd as well. I imagine the MIME type be deferred from the file extension, falling back to "application/octet-stream". I also imagine you being able to register a range of types with say "image/*" or similar.

See also https://lists.sr.ht/~eliasnaur/gio-patches/patches/42549.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But in that case what "mime" is the "scheme"? I mean, how I listen to all schemes?MIME: "schemes/*? How I listen to a specific scheme? MIME: "schemes/myCustomScheme?

Also, what happens when the app is launched using one URL/Scheme? Because, in that specific case, I think you don't have TargetOp setup yet. For each new TargetOp it sends the "startup-url"?

Copy link
Contributor

Choose a reason for hiding this comment

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

But in that case what "mime" is the "scheme"? I mean, how I listen to all schemes?MIME: "schemes/*? How I listen to a specific scheme? MIME: "schemes/myCustomScheme?

Good points. A SchemeOp seems appropriate.

Also, what happens when the app is launched using one URL/Scheme? Because, in that specific case, I think you don't have TargetOp setup yet. For each new TargetOp it sends the "startup-url"?

The incoming URL must be retined by the app package until one frame has been processed, to give the program a chance to receive the URL. After that, it can be forgotten.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I add a new commit with SchemeOp. I create a another router/transfer.go, since the current TargetOpseems to be integrated with clip-area.

Comment on lines 1065 to 1189
mmap, err := windows.OpenFileMapping(windows.FILE_MAP_WRITE, schemesURI)
if err != nil {
return
}
defer windows.CloseFileMapping(mmap)

dst, err := windows.MapViewOfFile(mmap, windows.FILE_MAP_WRITE, 1024*8)
if err != nil {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think WM_COPYDATA is simpler and safer than communicating through the filesystem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to use WM_COPYDATA but it requires to know the specific HWND of the receiver. Also, in both cases it will need to have mutex, which named-mmap is also used for.

The receiver creates the mmap using the scheme as the name of the mmap. The sender checks if the mmap already exists, if exists writes the URL into the mmap and then sends a event, that event don't contain any pointer or mmap.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to use WM_COPYDATA but it requires to know the specific HWND of the receiver.

If Windows opens a particular Gio binary, you should be able to locate any other running instances of that binary and only target them with WM_COPYDATA. For example, by the window class name or window name/id.

Also, in both cases it will need to have mutex, which named-mmap is also used for.

See above, I don't think you need a mutex if you just target windows belonging to the binary being opened.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found one issue related to UNC. I tried to search by path, then send COPYDATA to that executable.

For instance, it's possible to have one X:/demo.exe and //192.168.0.1/demo.exe, the first one is one "map" to the second one. Here is the issue, because I can't use os.Executable() == anotherExePath.

Because of that edge-case, I'm comparing the filepath.Base. However, Base may cause collision (for instance your/app.exe is the same of randompath/app.exe).

I think another alternative is to use -appid (?) from gogio and also checks the WindowClassName (which is currently hardcoded as "Gio"), not just the filepath.Base.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think another alternative is to use -appid (?) from gogio and also checks the WindowClassName (which is currently hardcoded as "Gio"), not just the filepath.Base.

Yes, please. That's what I meant by my comment mentioning window class names. I really don't think we should use executable paths to identify scheme event receivers. Coupled with my review comment about RegisterScheme belonging in gogio or an installer, I think gogio to make URL scheme opening work on Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I notice that appID is set in internal/log, and used only for Android. I create a new variable, in os_windows.go, but seems that both have the same purpose.

Maybe we should create some app/meta/meta.go which contains AppID, Version, URLSchemes (...)? That makes easier to set variables (without gogio) and also re-use it anywhere.

I will leave as duplicated, and in some next patch create such package.

Now, it's possible to launch one Gio app using a custom URI scheme, such as `gio://some/data`.

This feature is supported on Android, iOS, macOS and Windows, issuing a new transfer.URLEvent,
containing the URL launched. If the program is already open, one transfer.URLEvent will be
sent to the current  app.

Limitations:
On Windows, if the program listen to schemes (compiled with `-schemes`), then just a single
instance of the app can be open. In other words, just a single `myprogram.exe` can
be active.

Security:
Deeplinking have the same level of security of clipboard. Any other software can send such
information and read the content, without any restriction. That should not be used to transfer
sensible data, and can't be fully trusted.

Setup/Compiling:
In order to set the custom scheme, you need to use the new `-schemes` flag in `gogio`, using
as `-schemes gio` will listen to `gio://`.

If you are not using gogio you need to defined some values, which varies for each OS:

macOS/iOS - You need to define the following Properly List:
```
<key>CFBundleURLTypes</key>
<array>
  <dict>
        <key>CFBundleURLSchemes</key>
        <array>
          <string>yourCustomScheme</string>
        </array>
  </dict>
</array>
```

Windows - You need to compiling using -X argument:
```
-ldflags="-X "gioui.org/app.schemesDeeplink=yourCustomScheme" -H=windowsgui"
```

Android - You need to add IntentFilter in GioActivity:
```
<intent-filter>
        <action android:name="android.intent.action.VIEW"></action>
        <category android:name="android.intent.category.DEFAULT"></category>
        <category android:name="android.intent.category.BROWSABLE"></category>
        <data android:scheme="yourCustomScheme"></data>
</intent-filter>
```

That assumes that you still using GioActivity and GioAppDelegate, otherwise more
changes are required.

Signed-off-by: inkeliz <[email protected]>
@inkeliz
Copy link
Contributor Author

inkeliz commented Jun 3, 2024

@eliasnaur I need to squash multiple commits, due to rebase. I test have tested it on macOS, Android, iOS and Windows.

I tried to avoid re-introducing the old views map/list, but since the URL is received from the main app, not to a specific view, I didn't find a better way to route such event. :\

Copy link
Contributor

@eliasnaur eliasnaur left a comment

Choose a reason for hiding this comment

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

Looks great overall. A few nits and questions in the comments.

@@ -320,6 +329,12 @@ func (p *pointerFilter) Matches(e event.Event) bool {
return true
}
}
case transfer.URLEvent:
for _, t := range p.scheme {
if e.URL != nil && (t == "" || t == e.URL.Scheme) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should assume e.URL != nil.

Comment on lines +218 to +219
case transfer.URLFilter:
t = q // See comment in processEvent.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not treat transfer.URLFilter the same as the tag-less key.Filter above, instead of inventing a fake tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because q.key.scratchFilter = append(q.key.scratchFilter, f) is a specific slice that holds key.Filter. Where the "generic one" (used by anything else) uses "taggedFilter". I don't think is right to add yet another specialised slice.

@@ -464,6 +468,10 @@ func (q *Router) processEvent(e event.Event, system bool) {
cstate, evts := q.cqueue.Push(state.clipboardState, e)
state.clipboardState = cstate
q.changeState(e, state, evts)
case transfer.URLEvent:
var evts []taggedEvent
evts = append(evts, taggedEvent{tag: q, event: e})
Copy link
Contributor

Choose a reason for hiding this comment

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

Like key.Event above, I would not expect a transfer.URLEvent to have an associated tag.

// On Windows, launching the app using a URI will start a new instance of the app,
// a new window. That behavior, by default, doesn't align with iOS/Android/macOS, where
// the deeplink sends the event to the running app (if any). We are emulating it.
if hwnd, _ := windows.FindWindow(ID); hwnd != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if multiple program instances have windows matching the ID?

Copy link
Contributor Author

@inkeliz inkeliz Jun 8, 2024

Choose a reason for hiding this comment

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

I think that is unlikely, and only one instance of the program must run anyway. Previously the executable path was used as identifier, then moved to appid (#117 (comment)).

Maybe in gogio it must throw one error if appid is not provided and schemes are.

Assuming that appid is unique only one instance of this app will run.

@inkeliz inkeliz requested a review from eliasnaur June 9, 2024 15:09
@eliasnaur
Copy link
Contributor

I wanted to merge this soon, and thought more about the corner-cases. I'm having cold feet with respect to exposing deeplinking support as io/transfer.URLFilter, for two reasons:

  1. Deeplink events feels like something handled by the app, not any particular window event loop.
  2. More importantly, programs on the macOS platform may not even have any windows open to handle the deeplink event. In other words, you want to open a new window when a deeplink event arrives, but there's no open windows to detect it.

I don't have better ideas than to back out the io/transfer changes and add a app.Events call:

package app

// Events is an iterator that yields events that are not specific to any window,
// such as [URLEvent]. It never returns.
//
// Events must be called by the main goroutine, and replaces the
// call to [Main].
func Events(yield func(event.Event) bool)

and app.Main rewritten to:

package app

func Main() {
    Events(func(event.Event){})
}

(app.Main will probably be removed in a future version).

@eliasnaur
Copy link
Contributor

One consequence of a global app.Events is that the Windows implementation probably needs to create and drive a hidden window just for the purpose of receiving deeplink events.

@inkeliz
Copy link
Contributor Author

inkeliz commented Jun 18, 2024

Originally, the event was sent directly to window.Event (instead of gtx.Event).

I find it odd to create another event handler (window.Event, layout.Context.Event, and now app.Events). While I understand the reasoning, since this Event is not associated with a specific Window, I think it's just too much. Also, some OSes (Android and Switch) it's somewhat bounded to the View/Window.

@whereswaldon whereswaldon force-pushed the main branch 2 times, most recently from f8029f2 to 026d3f9 Compare June 20, 2024 07:54
@eliasnaur
Copy link
Contributor

How do you propose delivering the deeplink events to a running programs without open windows?

@whereswaldon whereswaldon force-pushed the main branch 13 times, most recently from 3d36537 to 74ccc9c Compare June 27, 2024 14:39
@barrowkwan
Copy link

does gio community consider to merge this to main? I am testing out golang to write mobile app and working on an Oauth ( keycloak ) authentication which will do a "callback", normally we will define custom scheme ( eg myapp://callback ) to receive the token back. it will be helpful to have the deeplink support.

@inkeliz is it easy to make the deeplink as a separate package ?

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