Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 24 additions & 6 deletions ScreenTranslate/Features/Capture/CaptureManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,27 @@ actor CaptureManager {
// MARK: - Permission Handling

/// Checks if the app has screen recording permission.
/// Uses CGPreflightScreenCaptureAccess() which does NOT trigger dialog.
/// - Returns: True if permission is granted
/// Uses SCShareableContent to actually verify permission works (not just cached status).
/// - Returns: True if permission is granted and functional
var hasPermission: Bool {
get async {
// Quick check first
guard CGPreflightScreenCaptureAccess() else {
return false
}
// Actually verify by trying to get shareable content
do {
_ = try await SCShareableContent.current
return true
} catch {
return false
}
Comment on lines +55 to +60
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n ScreenTranslate/Features/Capture/CaptureManager.swift | sed -n '40,70p'

Repository: hubo1989/ScreenTranslate

Length of output: 1254


🏁 Script executed:

cat -n ScreenTranslate/Features/Capture/CaptureManager.swift | sed -n '85,100p'

Repository: hubo1989/ScreenTranslate

Length of output: 745


🏁 Script executed:

cat -n ScreenTranslate/Features/Capture/CaptureManager.swift | sed -n '155,170p'

Repository: hubo1989/ScreenTranslate

Length of output: 712


🏁 Script executed:

rg -n "hasPermission|permissionDenied|SCShareableContent\.current" ScreenTranslate/Features/Capture/CaptureManager.swift

Repository: hubo1989/ScreenTranslate

Length of output: 470


🏁 Script executed:

cat -n ScreenTranslate/Features/Capture/CaptureManager.swift | sed -n '195,260p'

Repository: hubo1989/ScreenTranslate

Length of output: 3136


🏁 Script executed:

rg -n "getSCDisplay|throw|catch" ScreenTranslate/Features/Capture/CaptureManager.swift | head -20

Repository: hubo1989/ScreenTranslate

Length of output: 1374


🏁 Script executed:

rg -n "ScreenTranslateError" ScreenTranslate/Features/Capture/CaptureManager.swift

Repository: hubo1989/ScreenTranslate

Length of output: 1103


hasPermission 将所有异常统一映射为 false,掩盖实际错误信息

第 55-60 行的 catch 块捕获 SCShareableContent.current 的所有异常并无区别地返回 false,导致第 92 行和 161 行的调用方都将其解释为权限拒绝(permissionDenied)。这会将网络错误、系统级故障等非权限问题误报为权限问题。

对比 getSCDisplay 方法(第 246-258 行)的做法,它在同一调用上使用了正确的错误处理模式:throw ScreenTranslateError.captureFailure(underlying: error),保留底层错误信息供调用方判断。建议 hasPermission 也按此模式优化,区分权限错误和其他故障,避免错误信息丢失。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ScreenTranslate/Features/Capture/CaptureManager.swift` around lines 55 - 60,
The hasPermission method currently swallows all errors from awaiting
SCShareableContent.current and always returns false, which hides non-permission
failures; change hasPermission to rethrow the underlying error instead of
returning false for all exceptions (mirror the pattern used in getSCDisplay),
i.e., catch errors from SCShareableContent.current and throw a
ScreenTranslateError.captureFailure(underlying: error) (or only map genuine
permission-denied errors to false) so callers of hasPermission (the code paths
that interpret its result as permissionDenied) can distinguish network/system
failures from actual permission denial.

}
}

/// Synchronous permission check using only CGPreflightScreenCaptureAccess.
/// Use only when async check is not possible.
var hasPermissionSync: Bool {
CGPreflightScreenCaptureAccess()
}

Expand All @@ -70,8 +88,8 @@ actor CaptureManager {
isCapturing = true
defer { isCapturing = false }

// Check permission
guard hasPermission else {
// Check permission using async method
guard await hasPermission else {
throw ScreenTranslateError.permissionDenied
}

Expand Down Expand Up @@ -139,8 +157,8 @@ actor CaptureManager {
isCapturing = true
defer { isCapturing = false }

// Check permission
guard hasPermission else {
// Check permission using async method
guard await hasPermission else {
throw ScreenTranslateError.permissionDenied
}

Expand Down
38 changes: 17 additions & 21 deletions ScreenTranslate/Features/Onboarding/OnboardingViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -144,30 +144,26 @@ final class OnboardingViewModel {
func checkPermissions() {
hasAccessibilityPermission = AccessibilityPermissionChecker.hasPermission

// Check screen recording permission using multiple methods for reliability
hasScreenRecordingPermission = checkScreenRecordingPermission()
// Check screen recording permission using async method
Task {
hasScreenRecordingPermission = await checkScreenRecordingPermission()
}
}

/// Checks screen recording permission using multiple methods for reliability
private func checkScreenRecordingPermission() -> Bool {
// Method 1: CGPreflightScreenCaptureAccess (may not work in all cases)
if CGPreflightScreenCaptureAccess() {
return true
/// Checks screen recording permission using ScreenCaptureKit for reliable detection
private func checkScreenRecordingPermission() async -> Bool {
// First do a quick check with CGPreflightScreenCaptureAccess
if !CGPreflightScreenCaptureAccess() {
return false
}

// Method 2: Check if we can see windows from other apps
// If we have permission, we should see windows from other apps
let windowList = CGWindowListCopyWindowInfo([.optionOnScreenOnly], kCGNullWindowID) as? [[String: Any]] ?? []
let ownPID = ProcessInfo.processInfo.processIdentifier

// Count windows from other processes
let otherAppWindows = windowList.filter { window in
guard let ownerPID = window[kCGWindowOwnerPID as String] as? Int32 else { return false }
return ownerPID != ownPID
// Verify by actually trying to get shareable content
do {
_ = try await SCShareableContent.current
return true
} catch {
return false
}

// If we can see windows from other apps, we likely have permission
return otherAppWindows.count > 3
}

/// Requests screen recording permission
Expand Down Expand Up @@ -234,8 +230,8 @@ final class OnboardingViewModel {

switch type {
case .screenRecording:
// Use multiple methods to check permission without triggering dialog
let granted = checkScreenRecordingPermission()
// Use async ScreenCaptureKit check for reliable detection
let granted = await checkScreenRecordingPermission()
if granted {
hasScreenRecordingPermission = true
permissionCheckTask = nil
Expand Down
Loading