From 7ac7b9a678e5e3f7436eb9653a1f9109f914446d Mon Sep 17 00:00:00 2001 From: Ettore Pasquini Date: Thu, 1 Feb 2024 15:46:09 -0800 Subject: [PATCH 1/4] IOS-631 Fix crash when Publication::baseURL is nil --- .../ReaderPresentation/ReaderError.swift | 11 ++++++----- .../ReadingFormats/EPUBModule.swift | 6 +++++- .../Reader2/UI/NYPLEPUBViewController.swift | 18 +++++++++++++----- Simplified/en.lproj/Localizable.strings | 2 +- Simplified/it.lproj/Localizable.strings | 2 +- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/Simplified/Reader2/ReaderPresentation/ReaderError.swift b/Simplified/Reader2/ReaderPresentation/ReaderError.swift index 8964ce221..b15171fb4 100644 --- a/Simplified/Reader2/ReaderPresentation/ReaderError.swift +++ b/Simplified/Reader2/ReaderPresentation/ReaderError.swift @@ -14,15 +14,16 @@ import Foundation enum ReaderError: LocalizedError { case formatNotSupported - case epubNotValid - + case epubNotValid(String) + var errorDescription: String? { switch self { case .formatNotSupported: return NSLocalizedString("The book you were trying to read is in an unsupported format.", comment: "Error message when trying to read a publication with a unsupported format") - case .epubNotValid: - return NSLocalizedString("The book you were trying to read is corrupted. Please try downloading it again.", comment: "Error message when trying to read an EPUB that is invalid") + case .epubNotValid(let errorCause): + return String.localizedStringWithFormat( + NSLocalizedString("The book you were trying to read is corrupted (%@). Please try downloading it again.", comment: "Error message when trying to read an EPUB that is invalid"), + errorCause) } } - } diff --git a/Simplified/Reader2/ReaderPresentation/ReadingFormats/EPUBModule.swift b/Simplified/Reader2/ReaderPresentation/ReadingFormats/EPUBModule.swift index 02f53bd1c..934efe77a 100644 --- a/Simplified/Reader2/ReaderPresentation/ReadingFormats/EPUBModule.swift +++ b/Simplified/Reader2/ReaderPresentation/ReadingFormats/EPUBModule.swift @@ -39,9 +39,13 @@ final class EPUBModule: ReaderFormatModule { initialLocation: Locator?) throws -> UIViewController { guard publication.metadata.identifier != nil else { - throw ReaderError.epubNotValid + throw ReaderError.epubNotValid("nil metadata id") } + guard publication.baseURL != nil else { + throw ReaderError.epubNotValid("nil baseURL") + } + let epubVC = NYPLEPUBViewController(publication: publication, book: book, initialLocation: initialLocation, diff --git a/Simplified/Reader2/UI/NYPLEPUBViewController.swift b/Simplified/Reader2/UI/NYPLEPUBViewController.swift index 4a6cc02ba..0a0baaae6 100644 --- a/Simplified/Reader2/UI/NYPLEPUBViewController.swift +++ b/Simplified/Reader2/UI/NYPLEPUBViewController.swift @@ -40,16 +40,21 @@ fileprivate protocol EPUBNavigatorViewControllerMaking { } fileprivate class EPUBNavigatorViewControllerFactory: EPUBNavigatorViewControllerMaking { + /// - Important: `publication.baseURL` MUST not be nil when invoking this + /// function. @available(*, deprecated, message: "To suppress this warning, cast to EPUBNavigatorViewControllerMaking protocol") fileprivate func make(publication: Publication, initialLocation: Locator?, resourcesServer: ResourcesServer, config: EPUBNavigatorViewController.Configuration) -> EPUBNavigatorViewController { - EPUBNavigatorViewController(publication: publication, - initialLocation: initialLocation, - resourcesServer: resourcesServer, - config: config) + + assert(publication.baseURL != nil, "Publication.baseURL MUST not be nil") + + return EPUBNavigatorViewController(publication: publication, + initialLocation: initialLocation, + resourcesServer: resourcesServer, + config: config) } } @@ -63,6 +68,8 @@ class NYPLEPUBViewController: NYPLBaseReaderViewController { let userSettings: NYPLR1R2UserSettings + /// - Important: `publication.baseURL` MUST not be nil when invoking this + /// initializer. init(publication: Publication, book: NYPLBook, initialLocation: Locator?, @@ -92,7 +99,8 @@ class NYPLEPUBViewController: NYPLBaseReaderViewController { preloadNextPositionCount: 0, debugState: true) - // when changing the type of the navigator, also change `epubNavigator` getter + // create navigator + assert(publication.baseURL != nil, "Publication.baseURL MUST not be nil when creating a EPUBNavigatorViewController") let factory: EPUBNavigatorViewControllerMaking = EPUBNavigatorViewControllerFactory() let navigator = factory.make(publication: publication, initialLocation: initialLocation, diff --git a/Simplified/en.lproj/Localizable.strings b/Simplified/en.lproj/Localizable.strings index 72a4d9ed5..2c7def146 100644 --- a/Simplified/en.lproj/Localizable.strings +++ b/Simplified/en.lproj/Localizable.strings @@ -196,7 +196,7 @@ "The book you were trying to open is invalid." = "The book you were trying to open is invalid."; "An error was encountered while trying to open this book." = "An error was encountered while trying to open this book."; "The book you were trying to read is in an unsupported format." = "The book you were trying to read is in an unsupported format."; -"The book you were trying to read is corrupted. Please try downloading it again." = "The book you were trying to read is corrupted. Please try downloading it again."; +"The book you were trying to read is corrupted (%@). Please try downloading it again." = "The book you were trying to read is corrupted (%@). Please try downloading it again."; // MARK: - Authentication generic "Authentication Expired" = "Authentication Expired"; diff --git a/Simplified/it.lproj/Localizable.strings b/Simplified/it.lproj/Localizable.strings index bf3eedab2..586e7b059 100644 --- a/Simplified/it.lproj/Localizable.strings +++ b/Simplified/it.lproj/Localizable.strings @@ -192,7 +192,7 @@ "The book you were trying to open is invalid." = "Il libro che stavi cercando di leggere è in un formato non valido."; "An error was encountered while trying to open this book." = "Si è verificato un errore durante l'apertura di questo libro."; "The book you were trying to read is in an unsupported format." = "Il libro che stavi cercando di aprire è in un formato non supportato."; -"The book you were trying to read is corrupted. Please try downloading it again." = "Il libro che stavi cercando di leggere è danneggiato. Prova a scaricarlo di nuovo."; +"The book you were trying to read is corrupted (%@). Please try downloading it again." = "Il libro che stavi cercando di leggere è danneggiato (%@). Prova a scaricarlo di nuovo."; // MARK: - Authentication generic "Authentication Expired" = "Credenziali di accesso scadute"; From 8253df8547ae976917936cb11cd6b4f73c264406 Mon Sep 17 00:00:00 2001 From: Ettore Pasquini Date: Thu, 1 Feb 2024 15:49:29 -0800 Subject: [PATCH 2/4] IOS-631 Fix neverending spinner after book opening errors --- Simplified/Book/UI/NYPLBookButtonsView.h | 3 ++- Simplified/Book/UI/NYPLBookButtonsView.m | 2 +- Simplified/Book/UI/NYPLBookCellDelegate.m | 24 +++++++++++-------- .../Book/UI/NYPLBookDetailViewController.m | 10 ++++---- Simplified/Book/UI/NYPLEPUBOpener.swift | 6 ++--- .../NYPLRootTabBarController+R2.swift | 4 ++-- .../ReaderPresentation/ReaderModule.swift | 11 +++++---- 7 files changed, 34 insertions(+), 26 deletions(-) diff --git a/Simplified/Book/UI/NYPLBookButtonsView.h b/Simplified/Book/UI/NYPLBookButtonsView.h index a9997c243..9322b7491 100644 --- a/Simplified/Book/UI/NYPLBookButtonsView.h +++ b/Simplified/Book/UI/NYPLBookButtonsView.h @@ -8,7 +8,8 @@ - (void)didSelectReturnForBook:(NYPLBook *)book; - (void)didSelectDownloadForBook:(NYPLBook *)book; -- (void)didSelectReadForBook:(NYPLBook *)book successCompletion:(void(^)(void))completion; +- (void)didSelectReadForBook:(NYPLBook *)book + completion:(void(^)(BOOL success))completion; @end diff --git a/Simplified/Book/UI/NYPLBookButtonsView.m b/Simplified/Book/UI/NYPLBookButtonsView.m index 0cdbd5d2d..ca5993c70 100644 --- a/Simplified/Book/UI/NYPLBookButtonsView.m +++ b/Simplified/Book/UI/NYPLBookButtonsView.m @@ -434,7 +434,7 @@ - (void)didSelectRead self.activityIndicator.center = self.readButton.center; [self updateProcessingState:YES]; [self.delegate didSelectReadForBook:self.book - successCompletion:^{ + completion:^(__unused BOOL success) { [self updateProcessingState]; }]; } diff --git a/Simplified/Book/UI/NYPLBookCellDelegate.m b/Simplified/Book/UI/NYPLBookCellDelegate.m index 9e91003dd..e8899c345 100644 --- a/Simplified/Book/UI/NYPLBookCellDelegate.m +++ b/Simplified/Book/UI/NYPLBookCellDelegate.m @@ -63,7 +63,8 @@ - (void)didSelectDownloadForBook:(NYPLBook *)book [[NYPLMyBooksDownloadCenter sharedDownloadCenter] startDownloadForBook:book]; } -- (void)didSelectReadForBook:(NYPLBook *)book successCompletion:(void(^)(void))successCompletion +- (void)didSelectReadForBook:(NYPLBook *)book + completion:(void(^)(BOOL success))completion { #if FEATURE_DRM_CONNECTOR // Try to prevent blank books bug @@ -81,37 +82,40 @@ - (void)didSelectReadForBook:(NYPLBook *)book successCompletion:(void(^)(void))s completion:^(BOOL isSignedIn) { if (isSignedIn) { dispatch_async(dispatch_get_main_queue(), ^{ - [self openBook:book successCompletion:successCompletion]; + [self openBook:book completion:completion]; // with ARC, retain the reauthenticator until we're done, then release it reauthenticator = nil; }); } }]; } else { - [self openBook:book successCompletion:successCompletion]; + [self openBook:book completion:completion]; } #else - [self openBook:book successCompletion:successCompletion]; + [self openBook:book completion:completion]; #endif//FEATURE_DRM_CONNECTOR } -- (void)openBook:(NYPLBook *)book successCompletion:(void(^)(void))successCompletion +- (void)openBook:(NYPLBook *)book completion:(void(^)(BOOL success))completion { [NYPLCirculationAnalytics postEvent:@"open_book" withBook:book]; switch (book.defaultBookContentType) { case NYPLBookContentTypeEPUB: - [[[NYPLEPUBOpener alloc] init] open:book successCompletion:successCompletion]; + [[[NYPLEPUBOpener alloc] init] open:book completion:completion]; break; case NYPLBookContentTypePDF: [self openPDF:book]; break; #if FEATURE_AUDIOBOOKS - case NYPLBookContentTypeAudiobook: - [self openAudiobook:book successCompletion:successCompletion]; + case NYPLBookContentTypeAudiobook: { + [self openAudiobook:book successCompletion:^{ + completion(YES); + }]; break; + } #endif - default: + case NYPLBookContentTypeUnsupported: [self presentUnsupportedItemError]; break; } @@ -181,7 +185,7 @@ - (void)didSelectCancelForBookDownloadingCell:(NYPLBookDownloadingCell *const)ce - (void)didSelectListenForBookDownloadingCell:(NYPLBookDownloadingCell *)cell { - [self didSelectReadForBook:cell.book successCompletion:nil]; + [self didSelectReadForBook:cell.book completion:nil]; } @end diff --git a/Simplified/Book/UI/NYPLBookDetailViewController.m b/Simplified/Book/UI/NYPLBookDetailViewController.m index a88752311..52a03fd39 100644 --- a/Simplified/Book/UI/NYPLBookDetailViewController.m +++ b/Simplified/Book/UI/NYPLBookDetailViewController.m @@ -139,15 +139,17 @@ - (void)didSelectDownloadForBook:(NYPLBook *)book } - (void)didSelectReadForBook:(NYPLBook *)book - successCompletion:(__unused void(^)(void))successCompletion + completion:(void(^)(BOOL success))completion { [[NYPLBookCellDelegate sharedDelegate] didSelectReadForBook:book - successCompletion:^{ + completion:^(BOOL success) { // dismiss ourselves if we were presented, since we want to show the ereader if (self.modalPresentationStyle == UIModalPresentationFormSheet) { - [self dismissViewControllerAnimated:true completion:successCompletion]; + [self dismissViewControllerAnimated:true completion:^{ + completion(success); + }]; } else { - successCompletion(); + completion(success); } }]; } diff --git a/Simplified/Book/UI/NYPLEPUBOpener.swift b/Simplified/Book/UI/NYPLEPUBOpener.swift index 08861406d..4976285bb 100644 --- a/Simplified/Book/UI/NYPLEPUBOpener.swift +++ b/Simplified/Book/UI/NYPLEPUBOpener.swift @@ -10,7 +10,7 @@ import Foundation class NYPLEPUBOpener: NSObject { @objc - func open(_ book: NYPLBook, successCompletion: @escaping () -> Void) { + func open(_ book: NYPLBook, completion: @escaping (_ success: Bool) -> Void) { let url = NYPLMyBooksDownloadCenter.shared()? .fileURL(forBookIndentifier: book.identifier) @@ -21,8 +21,8 @@ class NYPLEPUBOpener: NSObject { rootTabController?.presentBook(book, fromFileURL: url, syncPermission: syncPermission, - successCompletion: successCompletion) - + completion: completion) + rootTabController?.annotationsSynchronizer? .checkServerSyncStatus(settings: NYPLSettings.shared, syncPermissionGranted: syncPermission) { enableSync, error in diff --git a/Simplified/Reader2/ReaderPresentation/NYPLRootTabBarController+R2.swift b/Simplified/Reader2/ReaderPresentation/NYPLRootTabBarController+R2.swift index 08f65c2e6..037871a0d 100644 --- a/Simplified/Reader2/ReaderPresentation/NYPLRootTabBarController+R2.swift +++ b/Simplified/Reader2/ReaderPresentation/NYPLRootTabBarController+R2.swift @@ -12,7 +12,7 @@ import Foundation func presentBook(_ book: NYPLBook, fromFileURL fileURL: URL?, syncPermission: Bool, - successCompletion: (() -> Void)?) { + completion: ((_ success: Bool) -> Void)?) { guard let libraryService = r2Owner?.libraryService, let readerModule = r2Owner?.readerModule else { return } @@ -30,7 +30,7 @@ import Foundation syncPermission: syncPermission, deviceID: drmDeviceID, in: navVC, - successCompletion: successCompletion) + completion: completion) case .cancelled: // .cancelled is returned when publication has restricted access to its resources and can't be rendered NYPLErrorLogger.logError(nil, summary: "Error accessing book resources", metadata: [ diff --git a/Simplified/Reader2/ReaderPresentation/ReaderModule.swift b/Simplified/Reader2/ReaderPresentation/ReaderModule.swift index f34829f50..e41b4b031 100644 --- a/Simplified/Reader2/ReaderPresentation/ReaderModule.swift +++ b/Simplified/Reader2/ReaderPresentation/ReaderModule.swift @@ -41,7 +41,7 @@ protocol ReaderModuleAPI { syncPermission: Bool, deviceID: String?, in navigationController: UINavigationController, - successCompletion: (() -> Void)?) + completion: ((_ success: Bool) -> Void)?) } @@ -82,7 +82,7 @@ final class ReaderModule: ReaderModuleAPI { syncPermission: Bool, deviceID: String?, in navigationController: UINavigationController, - successCompletion: (() -> Void)?) { + completion: ((_ success: Bool) -> Void)?) { if delegate == nil { NYPLErrorLogger.logError(nil, summary: "ReaderModule delegate is not set") } @@ -102,7 +102,7 @@ final class ReaderModule: ReaderModuleAPI { formatModule: formatModule, positioningAt: initialLocator, in: navigationController, - successCompletion: successCompletion) + completion: completion) } } @@ -112,7 +112,7 @@ final class ReaderModule: ReaderModuleAPI { formatModule: ReaderFormatModule, positioningAt initialLocator: Locator?, in navigationController: UINavigationController, - successCompletion: (() -> Void)?) { + completion: ((_ success: Bool) -> Void)?) { do { let readerVC = try formatModule.makeReaderViewController( for: publication, @@ -126,9 +126,10 @@ final class ReaderModule: ReaderModuleAPI { readerVC.hidesBottomBarWhenPushed = true navigationController.pushViewController(readerVC, animated: true) - successCompletion?() + completion?(true) } catch { delegate?.presentError(error, from: navigationController) + completion?(false) } } } From 43530ab21c23002ffa9cbff94f73e75abc606953 Mon Sep 17 00:00:00 2001 From: Ettore Pasquini Date: Mon, 5 Feb 2024 12:01:16 -0800 Subject: [PATCH 3/4] Fix GitHub CI warnings for using set-output --- scripts/detect-app-to-build.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/detect-app-to-build.sh b/scripts/detect-app-to-build.sh index fb8c5aeef..043899e00 100755 --- a/scripts/detect-app-to-build.sh +++ b/scripts/detect-app-to-build.sh @@ -42,6 +42,6 @@ echo "** Version / build number changes **" echo "SimplyE: ($SIMPLYE_CHANGED)" echo "Open eBooks: ($OPENEBOOKS_CHANGED)" -echo "::set-output name=simplye_changed::$SIMPLYE_CHANGED" -echo "::set-output name=openebooks_changed::$OPENEBOOKS_CHANGED" - +# make values accessible to GitHub workflows +echo "simplye_changed=$SIMPLYE_CHANGED" >> $GITHUB_OUTPUT +echo "openebooks_changed=$OPENEBOOKS_CHANGED" >> $GITHUB_OUTPUT From 0be29aeb8970fc9cde2567c0a65cb6e0857d4a71 Mon Sep 17 00:00:00 2001 From: Ettore Pasquini Date: Mon, 5 Feb 2024 14:29:51 -0800 Subject: [PATCH 4/4] IOS-631 Bump SimplyE build number to 3.9.6.0 --- Simplified.xcodeproj/project.pbxproj | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Simplified.xcodeproj/project.pbxproj b/Simplified.xcodeproj/project.pbxproj index 35678def3..7053658dd 100644 --- a/Simplified.xcodeproj/project.pbxproj +++ b/Simplified.xcodeproj/project.pbxproj @@ -5014,7 +5014,7 @@ CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES_ERROR; CODE_SIGN_ENTITLEMENTS = Simplified/SimplyE.entitlements; CODE_SIGN_IDENTITY = "iPhone Developer"; - CURRENT_PROJECT_VERSION = 4; + CURRENT_PROJECT_VERSION = 0; DEVELOPMENT_TEAM = 7262U6ST2R; "EXCLUDED_ARCHS[sdk=iphonesimulator*]" = arm64; GCC_PRECOMPILE_PREFIX_HEADER = YES; @@ -5034,7 +5034,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 3.9.5; + MARKETING_VERSION = 3.9.6; OTHER_LDFLAGS = ( "$(inherited)", "-ld_classic", @@ -5068,7 +5068,7 @@ CLANG_WARN_NON_LITERAL_NULL_CONVERSION = YES_ERROR; CODE_SIGN_ENTITLEMENTS = Simplified/SimplyE.entitlements; CODE_SIGN_IDENTITY = "iPhone Distribution"; - CURRENT_PROJECT_VERSION = 4; + CURRENT_PROJECT_VERSION = 0; DEVELOPMENT_TEAM = 7262U6ST2R; GCC_PRECOMPILE_PREFIX_HEADER = YES; GCC_PREFIX_HEADER = "Simplified/AppInfrastructure/Simplified-Prefix.pch"; @@ -5087,7 +5087,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 3.9.5; + MARKETING_VERSION = 3.9.6; OTHER_LDFLAGS = ( "$(inherited)", "-ld_classic",