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.

Sign up for GitHub

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

Jump to bottom

Replace legacy code on macOS builds #11428

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
+33 −21
Open

Replace legacy code on macOS builds #11428

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
316c5b0
Fix macOS build error due to CGDisplayStreamCreate being deprecated.
Nov 2, 2024
2b83e1b
change timeout of asynchronous callback to 5s
Nov 2, 2024
2dc39d1
Optimize code formatting.
Nov 2, 2024
d679b97
the macos version need to be checked during runtime
Nov 3, 2024
ab2e203
wrap unused parameter with Q_UNUSED macro
Nov 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ target_link_libraries(keepassxc_gui
${sshagent_LIB})

if(APPLE)
target_link_libraries(keepassxc_gui "-framework Foundation -framework AppKit -framework Carbon -framework Security -framework LocalAuthentication")
target_link_libraries(keepassxc_gui "-framework Foundation -framework AppKit -framework Carbon -framework Security -framework LocalAuthentication -framework ScreenCaptureKit")
if(Qt5MacExtras_FOUND)
target_link_libraries(keepassxc_gui Qt5::MacExtras)
endif()
Expand Down
2 changes: 1 addition & 1 deletion src/autotype/mac/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
set(autotype_mac_SOURCES AutoTypeMac.cpp)

add_library(keepassxc-autotype-cocoa MODULE ${autotype_mac_SOURCES})
set_target_properties(keepassxc-autotype-cocoa PROPERTIES LINK_FLAGS "-framework Foundation -framework AppKit -framework Carbon")
set_target_properties(keepassxc-autotype-cocoa PROPERTIES LINK_FLAGS "-framework Foundation -framework AppKit -framework Carbon -framework ScreenCaptureKit")
target_link_libraries(keepassxc-autotype-cocoa ${PROGNAME} Qt5::Core Qt5::Widgets)

install(TARGETS keepassxc-autotype-cocoa
Expand Down
49 changes: 30 additions & 19 deletions src/gui/osutils/macutils/AppKitImpl.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
#import "AppKitImpl.h"
#import <QWindow>
#import <Cocoa/Cocoa.h>
#if __clang_major__ >= 13 && MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_VERSION_12_3
#import <ScreenCaptureKit/ScreenCaptureKit.h>
#endif

@implementation AppKitImpl

Expand Down Expand Up @@ -181,28 +184,36 @@ - (bool) enableAccessibility
//
// Check if screen recording is enabled, may show an popup asking for permissions
//
- (bool) enableScreenRecording
- (bool) enableScreenRecording
{
#if __clang_major__ >= 9 && MAC_OS_X_VERSION_MIN_REQUIRED >= 1080
if (@available(macOS 10.15, *)) {
// Request screen recording permission on macOS 10.15+
// This is necessary to get the current window title
CGDisplayStreamRef stream = CGDisplayStreamCreate(CGMainDisplayID(), 1, 1, kCVPixelFormatType_32BGRA, nil,
^(CGDisplayStreamFrameStatus status, uint64_t displayTime,
IOSurfaceRef frameSurface, CGDisplayStreamUpdateRef updateRef) {
Q_UNUSED(status);
Q_UNUSED(displayTime);
Q_UNUSED(frameSurface);
Q_UNUSED(updateRef);
});
if (stream) {
CFRelease(stream);
} else {
return NO;
}
#if __clang_major__ >= 13 && MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_VERSION_12_3
if (@available(macOS 12.3, *)) {
__block BOOL hasPermission = NO;
dispatch_semaphore_t sema = dispatch_semaphore_create(0);

// Attempt to use SCShareableContent to check for screen recording permission
[SCShareableContent getShareableContentWithCompletionHandler:^(SCShareableContent * _Nullable content,
NSError * _Nullable error) {
Copy link
Member

Choose a reason for hiding this comment

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

You must use Q_UNUSED(error) for the second parameter. Otherwise the code will not compile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's compilable; otherwise, I wouldn't be able to test if the screen recording permission request dialog works properly. Additionally, the automated tests didn't report any errors. However, if you believe that adding Q_UNUSED(error) can improve the code's robustness, then it can certainly be included.

Copy link
Member

Choose a reason for hiding this comment

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

Q_UNUSED is only useful on Linux where GCC and Clang complain of unused parameters.

Copy link
Member

@varjolintu varjolintu Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I tested this with both macOS Sequoia and Sonoma. Disabled the setting in the System Preferences and then I didn't have a permission to perform Global Auto-Type.

Only complained about Q_UNUSED() because the compiler gives me an error, not a warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have you just disable the old permission or complete remove it? In my case macOS Sequoia just disable is not enough, I need to remove the keepassxc from privacy setting, then I see the dialog, after permit, the keepassxc is added to the list again.

image

Copy link
Member

Choose a reason for hiding this comment

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

If just disabling the setting does not work properly then there must be something missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's Macos problem, I have same story of some screenshot apps, I think it's about signature of software maybe....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested it, for the current official release 2.7.9, the behavior is the same, just disabling the setting does not work.

if (content) {
// Successfully obtained content, indicating permission is granted
hasPermission = YES;
} else {
// No permission or other error occurred
hasPermission = NO;
}
// Notify the semaphore that the asynchronous task is complete
dispatch_semaphore_signal(sema);
}];

// Wait for the asynchronous callback to complete
dispatch_time_t timeout = dispatch_time(DISPATCH_TIME_NOW, 5 * NSEC_PER_SEC);
dispatch_semaphore_wait(sema, timeout);

// Return the final result
return hasPermission;
}
#endif
return YES;
return YES; // Return YES for macOS versions that do not support ScreenCaptureKit
}

- (void) toggleForegroundApp:(bool) foreground
Expand Down