From 9dab07f41c93dba429d7cfc366f47646015dde9e Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Tue, 6 Apr 2021 14:28:03 +1000 Subject: [PATCH 1/7] set kConversationInitialMaxRangeSize back to 100 and fix the crash --- Session/Conversations/ConversationViewModel.m | 2 +- SignalUtilitiesKit/Utilities/ThreadUtil.m | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/Session/Conversations/ConversationViewModel.m b/Session/Conversations/ConversationViewModel.m index cee8ff67e..b893fe7f5 100644 --- a/Session/Conversations/ConversationViewModel.m +++ b/Session/Conversations/ConversationViewModel.m @@ -173,7 +173,7 @@ NS_ASSUME_NONNULL_BEGIN static const int kYapDatabasePageSize = 100; // Never show more than n messages in conversation view when user arrives. -static const int kConversationInitialMaxRangeSize = 25000; // TODO: Does it cause issues to set this so high? +static const int kConversationInitialMaxRangeSize = 100; // Never show more than n messages in conversation view at a time. static const int kYapDatabaseRangeMaxLength = 25000; diff --git a/SignalUtilitiesKit/Utilities/ThreadUtil.m b/SignalUtilitiesKit/Utilities/ThreadUtil.m index 68a52e67e..62a5963fe 100644 --- a/SignalUtilitiesKit/Utilities/ThreadUtil.m +++ b/SignalUtilitiesKit/Utilities/ThreadUtil.m @@ -198,14 +198,6 @@ NS_ASSUME_NONNULL_BEGIN visibleUnseenMessageCount++; interactionAfterUnreadIndicator = interaction; - - if (visibleUnseenMessageCount + 1 >= maxRangeSize) { - // If there are more unseen messages than can be displayed in the - // messages view, show the unread indicator at the top of the - // displayed messages. - *stop = YES; - hasMoreUnseenMessages = YES; - } }]; if (!interactionAfterUnreadIndicator) { From 513775b952df725ae76ba8956a2c777d3eac98fe Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Tue, 6 Apr 2021 14:32:55 +1000 Subject: [PATCH 2/7] add comments --- SignalUtilitiesKit/Utilities/ThreadUtil.m | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/SignalUtilitiesKit/Utilities/ThreadUtil.m b/SignalUtilitiesKit/Utilities/ThreadUtil.m index 62a5963fe..bb191b0db 100644 --- a/SignalUtilitiesKit/Utilities/ThreadUtil.m +++ b/SignalUtilitiesKit/Utilities/ThreadUtil.m @@ -169,6 +169,12 @@ NS_ASSUME_NONNULL_BEGIN // the messages view the position of the unread indicator, // so that it can widen its "load window" to always show // the unread indicator. + // + // We'll load all of the unread messages. This number may be larger + // than the maxRangeSize. This is not the idealest solution for the + // crash when unread message number is over 100. We can do better if + // we just load maxRangeSize number of unread messages, but the unread + // indicator can still be at the correct position. __block long visibleUnseenMessageCount = 0; __block TSInteraction *interactionAfterUnreadIndicator = nil; __block BOOL hasMoreUnseenMessages = NO; From df1b4fda5707ef52d922b99cbcb5b2cfc0bf798b Mon Sep 17 00:00:00 2001 From: ryanzhao Date: Wed, 7 Apr 2021 16:47:39 +1000 Subject: [PATCH 3/7] try to handle limited photo permission --- .../ConversationVC+Interaction.swift | 33 ++++++++++++++++--- Session/Meta/Session-Info.plist | 4 ++- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index a18f369f3..0560a7652 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -1,5 +1,6 @@ import CoreServices import Photos +import PhotosUI extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuActionDelegate, ScrollToBottomButtonDelegate, SendMediaNavDelegate, UIDocumentPickerDelegate, AttachmentApprovalViewControllerDelegate, GifPickerViewControllerDelegate, @@ -727,7 +728,34 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc } func requestLibraryPermissionIfNeeded() -> Bool { - switch PHPhotoLibrary.authorizationStatus() { + let authorizationStatus: PHAuthorizationStatus + if #available(iOS 14, *) { + authorizationStatus = PHPhotoLibrary.authorizationStatus(for: .readWrite) + if (authorizationStatus == .notDetermined) { + PHPhotoLibrary.requestAuthorization(for: .readWrite) { newStatus in + switch newStatus { + case .authorized: + print("RYAN: Full access.") + break + case .limited: + print("RYAN: Limited access.") + break + case .denied: + break + default: + break + } + } + return false + } + } else { + authorizationStatus = PHPhotoLibrary.authorizationStatus() + if (authorizationStatus == .notDetermined) { + PHPhotoLibrary.requestAuthorization { _ in } + return false + } + } + switch authorizationStatus { case .authorized, .limited: return true case .denied, .restricted: let modal = PermissionMissingModal(permission: "library") { } @@ -735,9 +763,6 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc modal.modalTransitionStyle = .crossDissolve present(modal, animated: true, completion: nil) return false - case .notDetermined: - PHPhotoLibrary.requestAuthorization { _ in } - return false default: return false } } diff --git a/Session/Meta/Session-Info.plist b/Session/Meta/Session-Info.plist index e05788065..e276c43a5 100644 --- a/Session/Meta/Session-Info.plist +++ b/Session/Meta/Session-Info.plist @@ -2,6 +2,8 @@ + PHPhotoLibraryPreventAutomaticLimitedAccessAlert + BuildDetails CarthageVersion @@ -59,7 +61,7 @@ NSContactsUsageDescription Signal uses your contacts to find users you know. We do not store your contacts on the server. NSFaceIDUsageDescription - Session's Screen Lock feature uses Face ID. + Session's Screen Lock feature uses Face ID. NSMicrophoneUsageDescription Session needs access to your microphone to record media. NSPhotoLibraryAddUsageDescription From 009f690f1e3d4271051895404af084e16a4d5982 Mon Sep 17 00:00:00 2001 From: Niels Andriesse Date: Thu, 8 Apr 2021 10:30:39 +1000 Subject: [PATCH 4/7] Clean --- SignalUtilitiesKit/Utilities/ThreadUtil.m | 6 ------ 1 file changed, 6 deletions(-) diff --git a/SignalUtilitiesKit/Utilities/ThreadUtil.m b/SignalUtilitiesKit/Utilities/ThreadUtil.m index bb191b0db..62a5963fe 100644 --- a/SignalUtilitiesKit/Utilities/ThreadUtil.m +++ b/SignalUtilitiesKit/Utilities/ThreadUtil.m @@ -169,12 +169,6 @@ NS_ASSUME_NONNULL_BEGIN // the messages view the position of the unread indicator, // so that it can widen its "load window" to always show // the unread indicator. - // - // We'll load all of the unread messages. This number may be larger - // than the maxRangeSize. This is not the idealest solution for the - // crash when unread message number is over 100. We can do better if - // we just load maxRangeSize number of unread messages, but the unread - // indicator can still be at the correct position. __block long visibleUnseenMessageCount = 0; __block TSInteraction *interactionAfterUnreadIndicator = nil; __block BOOL hasMoreUnseenMessages = NO; From cf148fe8459d64f08594c07a087a711b95af3f2a Mon Sep 17 00:00:00 2001 From: Ryan ZHAO Date: Thu, 8 Apr 2021 13:36:20 +1000 Subject: [PATCH 5/7] handle access to selected photos & optimise the process of request photo library permission --- .../ConversationVC+Interaction.swift | 55 ++++++++++--------- Session/Shared/OWSScreenLockUI.m | 4 ++ SessionMessagingKit/Utilities/Environment.h | 2 + SessionMessagingKit/Utilities/Environment.m | 1 + 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index 0560a7652..fa0ffdb84 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -92,12 +92,14 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc } func handleLibraryButtonTapped() { - // FIXME: We're not yet handling the case where the user only gives access to selected photos/videos - guard requestLibraryPermissionIfNeeded() else { return } - let sendMediaNavController = SendMediaNavigationController.showingMediaLibraryFirst() - sendMediaNavController.sendMediaNavDelegate = self - sendMediaNavController.modalPresentationStyle = .fullScreen - present(sendMediaNavController, animated: true, completion: nil) + requestLibraryPermissionIfNeeded { [weak self] in + DispatchQueue.main.async { + let sendMediaNavController = SendMediaNavigationController.showingMediaLibraryFirst() + sendMediaNavController.sendMediaNavDelegate = self + sendMediaNavController.modalPresentationStyle = .fullScreen + self?.present(sendMediaNavController, animated: true, completion: nil) + } + } } func handleGIFButtonTapped() { @@ -727,43 +729,46 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc } } - func requestLibraryPermissionIfNeeded() -> Bool { + func requestLibraryPermissionIfNeeded(handler: @escaping () -> Void) { let authorizationStatus: PHAuthorizationStatus if #available(iOS 14, *) { authorizationStatus = PHPhotoLibrary.authorizationStatus(for: .readWrite) if (authorizationStatus == .notDetermined) { - PHPhotoLibrary.requestAuthorization(for: .readWrite) { newStatus in - switch newStatus { - case .authorized: - print("RYAN: Full access.") - break - case .limited: - print("RYAN: Limited access.") - break - case .denied: - break - default: - break + // When the user chooses to select photos (which is the .limit status), + // the PHPhotoUI will present the picker view on the top of the front view. + // Since we have this ScreenLockUI showing when we request premissions, + // the picker view will be presented on the top of ScreenLockUI. + // However, the ScreenLockUI will dismiss with the permission request alert view, + // the picker view then will dissmiss, too. The selection process cannot be finished + // this way. So we add a flag (isRequestingPermission) to prevent the ScreenLockUI + // from showing when we request the photo library permission. + Environment.shared.isRequestingPermission = true + PHPhotoLibrary.requestAuthorization(for: .readWrite) { status in + Environment.shared.isRequestingPermission = false + if ([PHAuthorizationStatus.authorized, PHAuthorizationStatus.limited].contains(status)) { + handler() } } - return false } } else { authorizationStatus = PHPhotoLibrary.authorizationStatus() if (authorizationStatus == .notDetermined) { - PHPhotoLibrary.requestAuthorization { _ in } - return false + PHPhotoLibrary.requestAuthorization { status in + if (status == .authorized) { + handler() + } + } } } switch authorizationStatus { - case .authorized, .limited: return true + case .authorized, .limited: + handler() case .denied, .restricted: let modal = PermissionMissingModal(permission: "library") { } modal.modalPresentationStyle = .overFullScreen modal.modalTransitionStyle = .crossDissolve present(modal, animated: true, completion: nil) - return false - default: return false + default: return } } diff --git a/Session/Shared/OWSScreenLockUI.m b/Session/Shared/OWSScreenLockUI.m index 65bf6f28e..e7cb44121 100644 --- a/Session/Shared/OWSScreenLockUI.m +++ b/Session/Shared/OWSScreenLockUI.m @@ -347,6 +347,10 @@ NS_ASSUME_NONNULL_BEGIN OWSLogVerbose(@"desiredUIState: none 3."); return ScreenLockUIStateNone; } + + if (Environment.shared.isRequestingPermission) { + return ScreenLockUIStateNone; + } if (Environment.shared.preferences.screenSecurityIsEnabled) { OWSLogVerbose(@"desiredUIState: screen protection 4."); diff --git a/SessionMessagingKit/Utilities/Environment.h b/SessionMessagingKit/Utilities/Environment.h index 64d7ddc61..edbd77376 100644 --- a/SessionMessagingKit/Utilities/Environment.h +++ b/SessionMessagingKit/Utilities/Environment.h @@ -29,6 +29,8 @@ @property (nonatomic, readonly) OWSPreferences *preferences; @property (nonatomic, readonly) OWSSounds *sounds; @property (nonatomic, readonly) OWSWindowManager *windowManager; +// We don't want to cover the window when we request the photo library permission +@property (nonatomic, readwrite) BOOL isRequestingPermission; @property (class, nonatomic) Environment *shared; diff --git a/SessionMessagingKit/Utilities/Environment.m b/SessionMessagingKit/Utilities/Environment.m index e4abb4602..81a39ccf2 100644 --- a/SessionMessagingKit/Utilities/Environment.m +++ b/SessionMessagingKit/Utilities/Environment.m @@ -58,6 +58,7 @@ static Environment *sharedEnvironment = nil; _proximityMonitoringManager = proximityMonitoringManager; _sounds = sounds; _windowManager = windowManager; + _isRequestingPermission = false; return self; } From efb23a5cb3d066ada86d53e8f8bf3021900a0d4f Mon Sep 17 00:00:00 2001 From: Niels Andriesse Date: Thu, 8 Apr 2021 16:03:52 +1000 Subject: [PATCH 6/7] Clean --- .../ConversationVC+Interaction.swift | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index fa0ffdb84..c38f49c15 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -733,28 +733,30 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc let authorizationStatus: PHAuthorizationStatus if #available(iOS 14, *) { authorizationStatus = PHPhotoLibrary.authorizationStatus(for: .readWrite) - if (authorizationStatus == .notDetermined) { + if authorizationStatus == .notDetermined { // When the user chooses to select photos (which is the .limit status), // the PHPhotoUI will present the picker view on the top of the front view. - // Since we have this ScreenLockUI showing when we request premissions, - // the picker view will be presented on the top of ScreenLockUI. - // However, the ScreenLockUI will dismiss with the permission request alert view, - // the picker view then will dissmiss, too. The selection process cannot be finished + // Since we have the ScreenLockUI showing when we request premissions, + // the picker view will be presented on the top of the ScreenLockUI. + // However, the ScreenLockUI will dismiss with the permission request alert view, so + // the picker view then will dismiss, too. The selection process cannot be finished // this way. So we add a flag (isRequestingPermission) to prevent the ScreenLockUI // from showing when we request the photo library permission. Environment.shared.isRequestingPermission = true + SNAppearance.switchToImagePickerAppearance() PHPhotoLibrary.requestAuthorization(for: .readWrite) { status in + SNAppearance.switchToSessionAppearance() Environment.shared.isRequestingPermission = false - if ([PHAuthorizationStatus.authorized, PHAuthorizationStatus.limited].contains(status)) { + if [ PHAuthorizationStatus.authorized, PHAuthorizationStatus.limited ].contains(status) { handler() } } } } else { authorizationStatus = PHPhotoLibrary.authorizationStatus() - if (authorizationStatus == .notDetermined) { + if authorizationStatus == .notDetermined { PHPhotoLibrary.requestAuthorization { status in - if (status == .authorized) { + if status == .authorized { handler() } } From cc2e43821f7b0970f29fe804c089206cf6386b00 Mon Sep 17 00:00:00 2001 From: Niels Andriesse Date: Thu, 8 Apr 2021 16:14:25 +1000 Subject: [PATCH 7/7] Quick fix dark mode issue --- .../ConversationVC+Interaction.swift | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Session/Conversations/ConversationVC+Interaction.swift b/Session/Conversations/ConversationVC+Interaction.swift index c38f49c15..785aa1235 100644 --- a/Session/Conversations/ConversationVC+Interaction.swift +++ b/Session/Conversations/ConversationVC+Interaction.swift @@ -729,7 +729,7 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc } } - func requestLibraryPermissionIfNeeded(handler: @escaping () -> Void) { + func requestLibraryPermissionIfNeeded(onAuthorized: @escaping () -> Void) { let authorizationStatus: PHAuthorizationStatus if #available(iOS 14, *) { authorizationStatus = PHPhotoLibrary.authorizationStatus(for: .readWrite) @@ -743,12 +743,18 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc // this way. So we add a flag (isRequestingPermission) to prevent the ScreenLockUI // from showing when we request the photo library permission. Environment.shared.isRequestingPermission = true - SNAppearance.switchToImagePickerAppearance() + let appMode = AppModeManager.shared.currentAppMode + // FIXME: Rather than setting the app mode to light and then to dark again once we're done, + // it'd be better to just customize the appearance of the image picker. There doesn't currently + // appear to be a good way to do so though... + AppModeManager.shared.setCurrentAppMode(to: .light) PHPhotoLibrary.requestAuthorization(for: .readWrite) { status in - SNAppearance.switchToSessionAppearance() + DispatchQueue.main.async { + AppModeManager.shared.setCurrentAppMode(to: appMode) + } Environment.shared.isRequestingPermission = false if [ PHAuthorizationStatus.authorized, PHAuthorizationStatus.limited ].contains(status) { - handler() + onAuthorized() } } } @@ -757,14 +763,14 @@ extension ConversationVC : InputViewDelegate, MessageCellDelegate, ContextMenuAc if authorizationStatus == .notDetermined { PHPhotoLibrary.requestAuthorization { status in if status == .authorized { - handler() + onAuthorized() } } } } switch authorizationStatus { case .authorized, .limited: - handler() + onAuthorized() case .denied, .restricted: let modal = PermissionMissingModal(permission: "library") { } modal.modalPresentationStyle = .overFullScreen