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

Double-click opening a file on macOS #1751

Closed
ArturKovacs opened this issue Nov 1, 2020 · 29 comments · Fixed by #3758
Closed

Double-click opening a file on macOS #1751

ArturKovacs opened this issue Nov 1, 2020 · 29 comments · Fixed by #3758
Labels
DS - macos S - enhancement Wouldn't this be the coolest?

Comments

@ArturKovacs
Copy link
Contributor

When a file is associated with an app on macOS, the application:openFile: or the application:openFiles: function gets called instead of passing the target filepath as an argument. The application must handle theese calls otherwise the os will display an error message to the user that the appilcation doesn't support opening the file type.

Winit currently doesn't seem to handle either of these and there doesn't seem to be any other way of making a winit application able to handle opening a file for when the user double-clicks on an associated file.

@ArturKovacs
Copy link
Contributor Author

ArturKovacs commented Nov 1, 2020

I'm willing to implement this but first it would be nice to get some feedback from someone who's made macOS apps that can open associated files and has an idea of how this should be implemented in winit. My plan is to handle both of application:openFile: and application:openFiles:, immediately responding to the os that the opening was successful (event if it isn't going to be) and then sending or queing a winit WindowEvent with the list of file paths. The event would be a new variant and would look something like the following.

pub enum WindowEvent {
    // ...
    
    /// A file was requested to be opened. This must be handled to allow a macOS app to open associated files that are
    /// double-clicked. Other systems usually pass the file path as an argument to the program. (See `std::env::args()`)
    OpenFiles(Vec<PathBuf>),
}

@ArturKovacs
Copy link
Contributor Author

@chrisduerr and @vbogaevsky as you are listed to be the contributors/maintainers for this platform, I'd like to ask you to give an opinion on the solution I suggested above. I'd be happy to tackle this 😊

@chrisduerr
Copy link
Contributor

I've never built any macOS applications, so I cannot tell you if the API is correct or even sensible to use. Though as stated previously it would be a pretty strange API for something like this, rather than an application reentry.

But I see no reason what would stop anyone from just sending a PR to add these APIs, it all seems very straight-forward to me.

@alvinhochun
Copy link
Contributor

Using event::WindowsEvent seems wrong as it doesn't seem like those events should be associated to a window. I wouldn't think adding a variant to event::Event is appropriate either, because it seems pretty much an event limited to macOS?

@kchibisov
Copy link
Member

I still don't understand what the point here. You track clicks and you have some sort of file view in your application, then you make this click and it goes through your ui view hierarchy. Then you check if you point on file and open a file, why winit should be even involved here, opening files in UI toolkit has nothing to do with winit imho.

@ArturKovacs
Copy link
Contributor Author

@alvinhochun Correct, these are limited to macOS as far as I can tell (Windows and GNOME will just forward the path as an argument and I assume so does every other Linux desktop envornment). If a variant shouldn't be added, what would be the solution you'd suggest?

@kchibisov No that's not what I'm trying to do. This refers to using the native file browser, "Finder" on macOS and double clicking there. The OS then figures out which application to open and the OS opens the application and calls this ominous application:openFile function.

@kchibisov
Copy link
Member

Can't you just get NSApplication and do whatever you want without touching winit?

@alvinhochun
Copy link
Contributor

@ArturKovacs I'm not sure if there are existing patterns in winit for handling platform-specific events.

@kchibisov My guess is that winit subclasses NSApplication in order to run its event loop, and the question would become something like "how to retrieve the NSApplication from winit" or "how to intercept platform-specific events/messages from winit".

@kchibisov
Copy link
Member

kchibisov commented Nov 3, 2020

@alvinhochun

fn hide_application(&self) {

If something like that works, I'm curious whether using that method will work for file opening.

@alvinhochun
Copy link
Contributor

Well, I was going to suggest adding EventLoopExtMacOS with a method to register a FnMut callback that is fired on application:openFile: and etc., but it seems a bit odd and I'm not entirely sure how it would work in practice (I guess the user might probably end up using EventLoopProxy to pass custom events).

@ArturKovacs
Copy link
Contributor Author

Thanks for the pointer @kchibisov, I'll try to experiment a bit to see if it's possible without having to add anything to winit.

Yeah the platform specific extension trait would definitely be less intrusive, I like that idea, @alvinhochun. What I'm a bit concerned about is the timing of things, specifically if it's going to be possible for the user to call the appropriate function in time to actually receive the callback. Although this can be mitigated by keeping the last openFile request stored so that the user doesn't miss it even if registering the callback later.

@ArturKovacs
Copy link
Contributor Author

I looked into this and as you probably know, winit defines the WinitWindowDelegate at runtime, during startup. What I would need is to add the twoapplication:openFile: methods to it. But I don't think it's possible after the class has been registered and instantiated so I just don't see a way for doing this without making modifications to winit.

Later I'll explore the possible implementation of an extension trait that allows registering a callback.

@kchibisov
Copy link
Member

Hm, maybe we should have some event loop extension builder, I'd need something like that for Wayland actually.

@ArturKovacs
Copy link
Contributor Author

You mean something like this?

enum Event {
   WindowEvent {...}
    // All the other events events 

    Extra(ExtraEvent)
}

// On Linux
enum WaylandEvent {
    ...
}
impl ExtraEventWaylandExt for ExtraEvent {
    fn into_wayland_event(self) -> WaylandEvent {...}
}

@kchibisov
Copy link
Member

No, for building event loop, not for events. Like you don't need event extension if you provide function pointers to winit when you're creating event loop or something like that.

@ArturKovacs
Copy link
Contributor Author

I see! Would that be identical to registering callback functions with the WindowBuilder with traits like the WindowBuilderExtUnix? Because that's what I was planning to do with this openFiles stuff.

@alvinhochun
Copy link
Contributor

What I'm a bit concerned about is the timing of things, specifically if it's going to be possible for the user to call the appropriate function in time to actually receive the callback.

I don't claim to know anything about GUI programming on macOS at all, but I think as long as the callback is registered before the EventLoop is run it would be fine. After all, the OS cannot hijack the program flow while the UI thread is running user code. The delegates should only be triggered when the event loop is running.

@ArturKovacs
Copy link
Contributor Author

I don't know why I wanted to place the function on the WindowBuilder but anyhow now it's on EventLoopWindowTarget so the function can be called on the event loop instance. I also created a small example repo where an app bundle is created so that this can be tested fairly easily. See #1759

@madsmtm
Copy link
Member

madsmtm commented Jul 6, 2023

Triage: The PR implementing this is several years old, and the approach was one I was never happy with (changing the application delegate class at runtime), since it is harder to ensure soundness, and it may interfere with downstream applications that want to override the application delegate themselves (e.g. accesskit does this for NSView currently, and I suspect this could become more common).

Instead, I think we should:

  • Create a subclass of the application delegate, using objc2::declare_class!.
  • Put the method on MacOSEventLoopBuilderExt, so that we can pick the desired application delegate classes at initialization time.
  • Make sure to pass in &EventLoopWindowTarget<T> to the callback (should improve ergonomics of actually doing something in the callback).

I will probably implement this myself at some point (if so, the I will assign myself to the issue), though my priorities are elsewhere at the moment, so if someone wants to do it first, feel free!

@madsmtm
Copy link
Member

madsmtm commented Aug 30, 2023

Likely that this will get easier after #2903, if so then I'll probably tackle it

@raphamorim
Copy link

That's great news @madsmtm! Thank you for the update and please let me know if I can help somehow like testing or anything.

@9mm
Copy link

9mm commented Jan 27, 2024

hey @madsmtm I am excited to access openFile: for use on Neovide. I notice you said this is blocked by #2903, which is blocked by #3367.

Now that #3367 is closed, I'm assuming all that's really left is #2903? Or has something changed since then.

@madsmtm
Copy link
Member

madsmtm commented Jan 28, 2024

I moved the relevant stuff from #3367 out into #3432, this is still blocked on that.

@madsmtm
Copy link
Member

madsmtm commented Jun 24, 2024

I ended up taking a completely different path with this: Instead of trying to expose all the functionality on NSApplicationDelegate via. Winit, I found out that we can use NSNotification such that Winit no longer needs control over the application delegate.

So after #3758, listening to these events will be possible by implementing a custom NSApplicationDelegate yourself using objc2::declare_class!, Objective-C, Swift, or similar. The WIP docs for winit::platform::macos should contain a rough example of how to do this once the PR is merged. Feel free to open an issue, probably preferably on the objc2 repo, if you're having trouble with doing so.

(Wow, it really was a rollercoaster getting this done!)

@Korne127
Copy link

That's awesome! I'm glad to hear that :)
Thank you for all your work on that :)

Btw, I coincidentally spent the last days working on a native macOS menu bar with winit.
Just for clarity, is the custom NSApplicationDelegate also necessary to connect menu bar selectors to specific functions like it's done here (madsmtm/objc2#614) (or is there also a different way to do that)? In that case, this would also allow native macOS menu bars with winit 😄

@madsmtm
Copy link
Member

madsmtm commented Jun 24, 2024

Btw, I coincidentally spent the last days working on a native macOS menu bar with winit.

Beware that I'm still not convinced that we should do native menu bars in Winit, see if the muda crate can cover your needs instead! (Just so that you don't do work that will end up being rejected)

@Korne127
Copy link

Oh no, I just meant a project using winit which should also get a native menubar, not contributing to winit itself! Sorry for the confusion 😅
This issue is probably not the right place, sorry for that, I just immediately thought of that because you also use NSApplicationDelegates for that (at least with how it's done in that issue).
But thanks for the tip, I appreciate it!

@madsmtm
Copy link
Member

madsmtm commented Jun 24, 2024

Oh no, I just meant a project using winit which should also get a native menubar, not contributing to winit itself! Sorry for the confusion 😅

No problem! The short answer though is that listening for menu bar events was possible before too, you just had to use any object, it didn't have to be the application delegate.

I just immediately thought of that because you also use NSApplicationDelegates for that (at least with how it's done in that issue).

Yeah, I can see how that might've been a bit confusing - I really should write some better introductory docs on this in objc2!

@Korne127
Copy link

Thanks for the explanation! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DS - macos S - enhancement Wouldn't this be the coolest?
8 participants