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 .isAccessGranted() method #90

Closed
wants to merge 20 commits into from
Closed

Conversation

Mubramaj
Copy link

Added new method isAccessGranted that will return true if the app has the right access to perform the active window

This method is useful on MAC to avoid getting the activeWin if the access are not granted.

In my case I use activeWin in an Interval to check the foreground window. Without this method the dialogs to ask for permission are prompted again and again.

I don't know if it was in your road map but I am opening this pull request anyway.

…pp has the right access to perform the active window

fixed typo

removed unused console log
@sindresorhus
Copy link
Owner

There are multiple types of accesses. Do you think it's enough to just expose one method for all of them?

@Mubramaj
Copy link
Author

Mubramaj commented Nov 23, 2020

You are right. There is 3 accesses right ?

I can work on this. Should I do 3 different methods for each access or one method that returns data for the 3 accesses ?

@sindresorhus
Copy link
Owner

I mean, can you imagine needing any of the specific ones instead of just all? I would prefer to keep it simple and do one method for all, but I'm curious if I'm missing any use-cases.

@Mubramaj
Copy link
Author

Maybe some apps ask for the Accessibility permission during the Installation process but not the screen recording. Or the other way around.

I think making the method more general and returning a JSON with all the permission can be useful.

I will update my pull request

@sindresorhus
Copy link
Owner

Then I think there should also be an .all property for ease of use.

Package.swift Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
Sources/is-access-granted/main.swift Outdated Show resolved Hide resolved
Sources/is-access-granted/main.swift Outdated Show resolved Hide resolved
Sources/is-access-granted/main.swift Outdated Show resolved Hide resolved
Sources/is-access-granted/main.swift Outdated Show resolved Hide resolved
@sindresorhus sindresorhus changed the title feat: Added new method isAccessGranted Add .isAccessGranted() method Nov 24, 2020
Mubramaj and others added 3 commits November 24, 2020 19:47
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
@Mubramaj
Copy link
Author

Then I think there should also be an .all property for ease of use.

You mean in the object returned by the isAccessGranted ?

By the way should I keep this name isAccessGranted , I think getPermissionStatuses is better

@sindresorhus
Copy link
Owner

You mean in the object returned by the isAccessGranted ?

Yes

By the way should I keep this name isAccessGranted , I think getPermissionStatuses is better

I think isAccessGranted makes sense if the return value is:

{
  all: false,
  screen: true,
  accessibility: false
}

@Mubramaj
Copy link
Author

I pushed the latest changes.

However I noticed that one last access is used for the URL of the browser. For this I saw that an apple script was used tell app \"Safari\" to get URL of front document . Unfortunately, I did not found a way to get this access without prompting the use with a dialog.
Interesting article unfortunately not working in this case

Couple possibilities:

  • Do not handle the URL in the isAccessGranted Method
  • Handle it but isAccessGranted method will trigger the pop up only once, the first time it's ran

Also I do noticed an Error on Mojave: Error: dydl Library not loaded but I think you already addressed it on:
#53

@Mubramaj
Copy link
Author

Mubramaj commented Dec 7, 2020

Hi @sindresorhus any update on this pull request?

@sindresorhus
Copy link
Owner

index.d.ts Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
@Mubramaj Mubramaj marked this pull request as draft December 15, 2020 20:24

/**
Resolves all the statuses of the accesses needed to check the active win
Returns an object of type AccessResult
Copy link
Owner

Choose a reason for hiding this comment

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

This should use proper TS doc comment syntax for return value.

index.d.ts Outdated Show resolved Hide resolved
}
}
}
return false
Copy link
Owner

Choose a reason for hiding this comment

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

Use tab-indentation

}

// Don't check windows owned by this process
if windowProcessIdentifier == processIdentifier {
Copy link
Owner

Choose a reason for hiding this comment

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

Use guard.

*/
func isScreenRecordingGrantedNoDialog() -> Bool {
// The screen recording security was introduced in version 10.15 of MAC os
if #available(macOS 10.15, *) {
Copy link
Owner

Choose a reason for hiding this comment

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

Use guard. Applies to many other places.

* CREDITS: https://gist.github.com/soffes/da6ea98be4f56bc7b8e75079a5224b37
*/
func isScreenRecordingGrantedNoDialog() -> Bool {
// The screen recording security was introduced in version 10.15 of MAC os
Copy link
Owner

Choose a reason for hiding this comment

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

That's not how you write macOS.

index.js Outdated Show resolved Hide resolved
@@ -71,6 +71,14 @@ Returns an `Object` with the result, or `undefined` if there is no active window
- `url` *(string?)* - URL of the active browser tab if the active window is Safari, Chrome, Edge, or Brave *(macOS only)*
- `memoryUsage` *(number)* - Memory usage by the window owner process

### activeWin.isAccessGranted()

Check if the package has enough access to get the active window.
Copy link
Owner

Choose a reason for hiding this comment

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

This is not correct. It always has access, just not access to the window title, etc.


Check if the package has enough access to get the active window.

Returns `true` if there is enough access, `false` otherwise.
Copy link
Owner

Choose a reason for hiding this comment

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

This is outdated.

Mubramaj and others added 2 commits January 3, 2021 23:04
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Base automatically changed from master to main January 23, 2021 07:44
@sindresorhus
Copy link
Owner

Closing as this isn't moving forward and the PR is just too far from being mergable. Probably better to leave it to someone else.

@Mubramaj
Copy link
Author

Mubramaj commented Mar 7, 2021

I read the article you sent me and did more research.
I tried experimenting the method AEDeterminePermissionToAutomateTarget described on the article to see if use granted permission to be able to send message from current App to other app (Like safari or Chrome) to get current URL.
However when the app is not running the return status is process not found .

Again this permission is only used to get Browser URL from active browser.
In my opinion the the statuses for accessibility and screen Recording permissions are enough

If you agree on keeping only this 2 I can work on the remaining comments you left to finalize this PR

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.

None yet

2 participants