servo / core-foundation-rs

Rust bindings to Core Foundation and other low level libraries on Mac OS X and iOS
Other
987 stars 216 forks source link

ScreenCaptureAccess.preflight returns incorrect value for Intel based 64bit Mac #677

Closed Pranav2612000 closed 1 month ago

Pranav2612000 commented 3 months ago

The function ScreenCaptureAccess.preflight [ Ref ] returns incorrectly for Intel based 64 bit Macs.

On further inspection, the reason for this is that the external method CGPreflightScreenCaptureAccess returns a value which is not 1 ( even if the permissions have been given ). From my research, I've found that it returns a number whose lowest 8 bits are 0. So something like the following should work

image

That's what I'm doing in my fork here

This is working good enough for me. What do you think about this approach? Does it make sense? I'm open to submitting a PR, or if you have a better solution in mind, I'll love to implement it.

madsmtm commented 3 months ago

The correct way to work with external booleans is always to compare them against NO / 0. So rather, you should use != 0

Pranav2612000 commented 3 months ago

Yup @madsmtm . That was what I tried first, but I observed that CGPreflightScreenCaptureAccess returns a non zero value ( the last 8 bits of which are 0 ) if screen capture permissions are not present. So the function would always return true, which is not the correct behaviour.

Is it possible that it's something to do with Boolean being 1 byte by int32 being 4 bytes? Because the last 8 bits of the returned value are always 0000 0000 or 0000 0001

madsmtm commented 3 months ago

Oh... Actually, you're right, the issue is that the definition in Rust is using boolean_t instead of bool :/

Pranav2612000 commented 3 months ago

Yup. Thank you @madsmtm What do you think the next steps should be here? Happy to submit a PR

madsmtm commented 3 months ago

Can't answer that, not a maintainer here, sorry

jrmuizel commented 3 months ago

Please submit a PR

waywardmonkeys commented 1 month ago

I've doe what @madsmtm suggested as the fix (after verifying in the SDK headers) in #698.