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

Replace legacy code on macOS builds #11428

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

FischLu
Copy link
Contributor

@FischLu FischLu commented Nov 2, 2024

fix #11288, The CGDisplayStreamCreate function has been deprecated and the current code fails to compile on macOS 15 and above. Following Apple's official recommendations, this fix replaces CGDisplayStreamCreate with ScreenCaptureKit (MacOS 12.3+). Since ScreenCaptureKit is an asynchronous framework, necessary semaphore waiting has been added to the code. This change successfully triggers the screen recording permission request dialog and allows proper use of autotype when permission is granted. The code has been tested and confirmed working on macOS 15.0.1.

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

@droidmonkey
Copy link
Member

droidmonkey commented Nov 2, 2024

Omg thank you, I really didn't want to figure this one out.

@droidmonkey droidmonkey added this to the v2.7.10 milestone Nov 2, 2024
@droidmonkey droidmonkey changed the title fix #11288 Replace legacy code on macOS builds Nov 2, 2024

// 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

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.

@FischLu
Copy link
Contributor Author

FischLu commented Nov 4, 2024

After adding Q_UNUSED(error), the automated tests do not pass. You may check the reason.

@droidmonkey
Copy link
Member

The failures are ephermal and unrelated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed build on macOS with Xcode 16.0
3 participants