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

Finicky Does Not Retain 'Open in Background' Setting to Pass to Opening Browser #111

Closed
hisaac opened this issue May 6, 2020 · 11 comments
Closed

Comments

@hisaac
Copy link

hisaac commented May 6, 2020

It appears that when an URL is set to be opened in the background, that setting is not passed from Finicky to the opening browser.

I did a little digging in the code, and I wonder if it's because right now the browser's are being opened by Finicky using a shell command. (This is an assumption based on the getBrowserCommand() method in Browsers.swift. It looks like you're building a shell command in that method.)

I wonder if it might be better to use NSWorkspace's open(_:withApplicationAt:configuration:completionHandler:) method instead. It allows for you to specify the app to open the URL, and pass in an NSWorkspace.OpenConfiguration value, which has an activates property. Passing in a false value for activates should open the app in the background.

This is all assuming that Finicky is being opened in a similar way, and that it's possible to grab the activates property off of whatever is opening Finicky. I haven't investigated far enough to know if that's true or not.

@johnste
Copy link
Owner

johnste commented May 8, 2020

I remember Finicky used to handle this condition. I suspect I accidently removed it when I refactored something. I think it shouldn't be that hard to reimplement without having to refactor the way Finicky launches applications.

@hisaac
Copy link
Author

hisaac commented May 8, 2020

Excellent, yeah hopefully it's not too difficult.

@johnste johnste closed this as completed in 008e968 May 9, 2020
@hisaac
Copy link
Author

hisaac commented May 9, 2020

I just tested the changes in 008e968, but the issue persists for me. I'm trying to debug in Xcode to see if I can follow it. What method gets called when Finicky handles a URL? I set breakpoints in all of the methods in the AppDelegate, but none of them seem to get hit.

@johnste
Copy link
Owner

johnste commented May 9, 2020 via email

@johnste
Copy link
Owner

johnste commented May 9, 2020

If there still are issues I can probably look into it tomorrow

@johnste johnste reopened this May 9, 2020
@johnste
Copy link
Owner

johnste commented May 9, 2020

If xcode doesn't break anywhere maybe a copy of finicky is already runnning?

@hisaac
Copy link
Author

hisaac commented May 9, 2020

I'm using Reeder to open the links, with its "Open links in background" setting on. And the browser I'm using is Safari.

When I have the default browser set to Safari, it works as expected — the links are opened in Safari, but Safari is not activated and brought to the foreground. When I have the default browser set to Finicky, the links are opened in Safari, but Safari is activated and brought to the foreground.

I got the breakpoints in Xcode to work now now — not sure why it wasn't the first time. Where are you pulling the "open this URL in the background" info from? The NSAppleEventDescriptor that gets sent into the handleGetURLEvent function? I'm not familiar with how Apple Events work, so I'm not sure how to find if the "open in background" piece is getting passed successfully.

Also, here's my config file if that helps:

module.exports = {
	defaultBrowser: "Safari",
	handlers: [
		{
			// Open Spotify links in Spotify.app
			match: finicky.matchHostnames("open.spotify.com"),
			browser: "Spotify",
		},
		{
			// Open Apple Music links directly in Music.app
			match: finicky.matchHostnames([
				"music.apple.com",
				"geo.music.apple.com",
			]),
			browser: "Music",
		},
		{
			// Open Zoom links in the Zoom.app
			match: /zoom.us\/j\//,
			browser: "us.zoom.xos",
		},
	],
	rewrite: [
		{
			// Rewrite Apple Music links to use `itmss` as protocol
			match: finicky.matchHostnames([
				"music.apple.com",
				"geo.music.apple.com",
			]),
			url: ({ url }) => ({
				...url,
				protocol: "itmss"
			}),
		},
	],
};

@johnste
Copy link
Owner

johnste commented May 10, 2020

Thanks for the detailed info. I'll try to reproduce the problem now.

Finicky keeps track of if it is activated or not with these methods in AppDelegate.swift:

    func applicationDidBecomeActive(_: Notification) {
        isInBackground = false
    }

    func applicationDidResignActive(_: Notification) {
        isInBackground = true
    }

It then uses isInBackground and passes it down to configLoader.determineOpeningApp which uses it's isActivated parameter to set the behavior of the BrowserOpts representing each browser to potentially launch.

// Config.swift
let browser = try BrowserOpts(name: browserName, appType: appType!, openInBackground: openInBackground ?? !isActivated) 

@johnste
Copy link
Owner

johnste commented May 10, 2020

I believe I have an idea of what's happening. Finicky assumes it's active when it is first started, so if a url is opened in the background it would default to opening it in the foreground.

Do you still see the issue if you open and close the Finicky console window first?

There's no obvious solution for this I think, will have to look into handling Apple Events more.

@johnste
Copy link
Owner

johnste commented May 10, 2020

Ok, after doing some searching I found another method of checking the current active state of the application, (just NSApplication.shared.isActive) and using it to determine if the browser is to be opened in the bg or not.

I just pushed b23487e which has my new attempt at a fix. It seems to work well when I try it. I'm using your config and Vienna.

@hisaac
Copy link
Author

hisaac commented May 11, 2020

Nice, it works now! Thank you for the quick fix!

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

No branches or pull requests

2 participants