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

Add NSUserActivityTypeBrowsingWeb handling #3635

Merged
merged 2 commits into from
Aug 18, 2024

Conversation

mdbraber
Copy link
Contributor

To handle Handoff from a native app to a (default) browser NSUserActivityTypeBrowsingWeb needs to be handled. If a default browser supports this, Handoff links that have a webpageUrl can be opened by the default browser.

More info: https://developer.apple.com/library/archive/documentation/UserExperience/Conceptual/Handoff/AdoptingHandoff/AdoptingHandoff.html

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['pr-fix', 'pr-change', 'pr-feature', 'pr-maintenance']

@mdbraber
Copy link
Contributor Author

@latenitefilms not sure how to add a a label afterwards, but I guess this is a pr-feature

@latenitefilms
Copy link
Contributor

I believe only @cmsj and @asmagill can add labels and approve pull requests.

@asmagill
Copy link
Member

I am out of town at the moment, but I will try and carve out some time here in the next couple of days to see about moving some of these forward.

@mdbraber
Copy link
Contributor Author

@asmagill any chance of getting this merged?

@cmsj cmsj added the pr-feature Pull Request implementing a feature label Aug 5, 2024
if ([userActivity.activityType isEqualToString:NSUserActivityTypeBrowsingWeb]) {
[[NSWorkspace sharedWorkspace] openURL:userActivity.webpageURL];
}
return YES;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be unconditionally returning YES? The docs suggest that we probably ought to return YES only inside the if statement, and otherwise return NO, right?

Choose a reason for hiding this comment

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

@cmsj; if i were to correct this in a separate PR would that be ok? don't want to step on @mdbraber's toes, but also really want this in.

@megalithic
Copy link

@mdbraber this is a fantastic update and would solve a major url handling complaint when using hammerspoon as the default browser. Thanks for tackling this!

@megalithic
Copy link

@mdbraber any updates or thoughts on this?

@cmsj cmsj merged commit 08cb813 into Hammerspoon:master Aug 18, 2024
1 of 2 checks passed
@cmsj
Copy link
Member

cmsj commented Aug 18, 2024

@megalithic @mdbraber I made the change I think is correct and merged, it would be super helpful if someone could test the Development Build to check this is working. It should be available in an hour or so from: https://github.com/Hammerspoon/hammerspoon/actions/runs/10444554151

@megalithic
Copy link

@megalithic @mdbraber I made the change I think is correct and merged, it would be super helpful if someone could test the Development Build to check this is working. It should be available in an hour or so from: https://github.com/Hammerspoon/hammerspoon/actions/runs/10444554151

@cmsj thank you (also to @mdbraber); it works well! was able to handoff an ios brave browser tab to hammerspoon and the URL handlers for hammerspoon took over. 💯

Copy link

codecov bot commented Aug 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 28.12%. Comparing base (36c6495) to head (403df1f).
Report is 821 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3635      +/-   ##
==========================================
+ Coverage   27.61%   28.12%   +0.50%     
==========================================
  Files         180      191      +11     
  Lines       36181    43532    +7351     
==========================================
+ Hits         9993    12242    +2249     
- Misses      26188    31290    +5102     

@mdbraber
Copy link
Contributor Author

Great to see this merged while I was away :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature Pull Request implementing a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants