-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
fix(macos): properly handle accessibility permission #2508
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2508 +/- ##
=========================================
- Coverage 11.17% 7.88% -3.29%
=========================================
Files 100 90 -10
Lines 17310 15803 -1507
Branches 8069 7487 -582
=========================================
- Hits 1934 1246 -688
+ Misses 12671 12131 -540
+ Partials 2705 2426 -279
Flags with carried forward coverage won't be shown. Click here to find out more. |
Needs test coverage on the new methods |
If we are going to test this, this has to be tested only on CI, this cannot be tested on macports test action. |
2340497
to
062d1fb
Compare
Waiting for #2550 to create the tests |
062d1fb
to
c24e808
Compare
This needs to be updated and tested on macOS Sequoia before merging. |
d8d4ada
to
96505cc
Compare
Revisiting this PR, fixing conflicts first to get this on track |
Should I include a manual request in the web ui? Or the status saying whether the permission is given or not? @ReenigneArcher |
This sounds like a good idea... should it be a separate PR? |
@ReenigneArcher not sure. I may do it in the same, so I can improve the API around it once, cause it may change where it is declared, and as I have some code quality issues to review, I may do it in a single pass |
96505cc
to
915c7bf
Compare
915c7bf
to
a2e3b24
Compare
Quality Gate failedFailed conditions See analysis details on SonarQube Cloud Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE |
#include "misc.h" | ||
#include "permissions_manager.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#include "misc.h" | |
#include "permissions_manager.h" | |
#include "src/platform/macos/misc.h" | |
#include "src/platform/macos/permissions_manager.h" |
Should we be more explicit?
#include <Carbon/Carbon.h> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#include <Carbon/Carbon.h> | |
#include <Carbon/Carbon.h> |
// Even though the following two functions are available starting in macOS 10.15, they weren't | ||
// actually in the Mac SDK until Xcode 12.2, the first to include the SDK for macOS 11 | ||
#if __MAC_OS_X_VERSION_MAX_ALLOWED < 110000 // __MAC_11_0 | ||
// If they're not in the SDK then we can use our own function definitions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably drop this, our readme lists macOS 13+. Although I don't have a strong opinion on this if we want to keep it as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't paid enough attention to it, yeah I'll remove it.
Description
Check for macOS Accessibility permission to allow mouse and keyboard input events to work properly.
Issues Fixed or Closed
Type of Change
.github/...
)Checklist
Branch Updates
LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.