From fa7716b4c94500c630defb7ae87d0f34500904e1 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Tue, 8 Sep 2026 15:17:12 -0600 Subject: [PATCH 1/2] fix: import device photos under iOS Lockdown Mode Under Lockdown Mode the system's PhotosFileProvider extension performs a hardened full decode when it materializes a picked image, and is killed at its 20 MB memory limit. A 36 MP photo needs ~144 MB, so importing one fails outright: `loadFileRepresentation` returns `NSItemProviderError -1000` over `NSCocoaErrorDomain 4099`. When Lockdown Mode is on, present a library-backed `PHPickerViewController` and read the picked file with `PHAssetResourceManager`, which is serviced by `photolibraryd` and has no such cap. Outside Lockdown Mode nothing changes: the same permission-free picker, the same item-provider path, no prompt. The two routes are exclusive. A failure reading from the library is reported rather than retried through the item provider, which in Lockdown Mode is the thing that cannot serve the file. Limited Photos authorization is refused up front, because the picker shows the whole library whatever the app has been granted and would otherwise offer items it can't read. Builds on #25997, which made the failure graceful and added the detection primitive this gates on. --- .../Media/PhotoLibraryFileLoaderTests.swift | 140 +++++++++++++ .../Classes/Services/MediaImportService.swift | 5 + .../Media/ItemProviderMediaExporter.swift | 173 ++++++++++++---- .../Media/PhotoLibraryFileLoader.swift | 195 ++++++++++++++++++ .../AztecPostViewController.swift | 34 ++- .../SiteIconPickerPresenter.swift | 11 +- .../GutenbergMediaInserterHelper.swift | 8 +- .../GutenbergMediaPickerHelper.swift | 28 +-- .../Gravatar/AvatarMenuController.swift | 11 +- .../Helpers/MediaPickerController.swift | 2 +- .../Helpers/MediaPickerMenuController.swift | 9 +- .../Media/MediaPicker/MediaPicker.swift | 6 +- .../Menu/MediaPickerMenu+Photos.swift | 31 +-- .../Media/PHPickerController+Extensions.swift | 9 - .../ViewRelated/Media/PhotosPickerAsset.swift | 86 ++++++++ .../Media/PhotosPickerPresenter.swift | 175 ++++++++++++++++ .../SiteMediaAddMediaMenuController.swift | 16 +- 17 files changed, 809 insertions(+), 130 deletions(-) create mode 100644 Tests/KeystoneTests/Tests/Features/Media/PhotoLibraryFileLoaderTests.swift create mode 100644 WordPress/Classes/Utility/Media/PhotoLibraryFileLoader.swift create mode 100644 WordPress/Classes/ViewRelated/Media/PhotosPickerAsset.swift create mode 100644 WordPress/Classes/ViewRelated/Media/PhotosPickerPresenter.swift diff --git a/Tests/KeystoneTests/Tests/Features/Media/PhotoLibraryFileLoaderTests.swift b/Tests/KeystoneTests/Tests/Features/Media/PhotoLibraryFileLoaderTests.swift new file mode 100644 index 000000000000..15f3f604c96e --- /dev/null +++ b/Tests/KeystoneTests/Tests/Features/Media/PhotoLibraryFileLoaderTests.swift @@ -0,0 +1,140 @@ +import Foundation +import Photos +import Testing +import UniformTypeIdentifiers + +@testable import WordPress + +/// Covers the resource the loader picks out of an asset. Getting this wrong is silent — +/// the upload succeeds, it just carries the wrong file (an unedited original, or the +/// video half of a Live Photo). +struct PhotoLibraryFileLoaderResourceTests { + + // MARK: - Photos + + /// An edited photo carries both the render and the untouched original. The render is + /// what the picker itself would have produced, so it's what gets uploaded. + @Test func prefersTheEditedRenditionOverTheOriginalPhoto() { + let index = PhotoLibraryFileLoader.preferredResourceIndex( + in: [.photo, .adjustmentData, .fullSizePhoto], + mediaType: .image + ) + #expect(index == 2) + } + + /// An unedited photo has no `.fullSizePhoto`, so the original is the only rendition. + @Test func fallsBackToTheOriginalPhotoWhenThereIsNoEditedRendition() { + let index = PhotoLibraryFileLoader.preferredResourceIndex(in: [.photo], mediaType: .image) + #expect(index == 0) + } + + /// A Live Photo is an image asset that also carries the video half. Matching on the + /// asset's media type keeps the still image from being swapped for the movie. + @Test func picksTheStillImageOfALivePhoto() { + let index = PhotoLibraryFileLoader.preferredResourceIndex( + in: [.pairedVideo, .photo], + mediaType: .image + ) + #expect(index == 1) + } + + // MARK: - Videos + + @Test func prefersTheEditedRenditionOverTheOriginalVideo() { + let index = PhotoLibraryFileLoader.preferredResourceIndex( + in: [.video, .fullSizeVideo], + mediaType: .video + ) + #expect(index == 1) + } + + @Test func fallsBackToTheOriginalVideoWhenThereIsNoEditedRendition() { + let index = PhotoLibraryFileLoader.preferredResourceIndex(in: [.video], mediaType: .video) + #expect(index == 0) + } + + /// A video asset should never be uploaded as one of its still frames. + @Test func neverPicksAPhotoResourceForAVideo() { + let types: [PHAssetResourceType] = [.photo, .fullSizePhoto, .video] + let index = PhotoLibraryFileLoader.preferredResourceIndex(in: types, mediaType: .video) + #expect(index == 2) + } + + // MARK: - Fallbacks + + /// Nothing matched, so the first resource is the best guess — the same one the picker + /// would have handed over. + @Test func fallsBackToTheFirstResourceWhenNoPreferredTypeIsPresent() { + let index = PhotoLibraryFileLoader.preferredResourceIndex( + in: [.adjustmentData, .adjustmentBasePhoto], + mediaType: .unknown + ) + #expect(index == 0) + } + + @Test func returnsNilWhenTheAssetHasNoResources() { + #expect(PhotoLibraryFileLoader.preferredResourceIndex(in: [], mediaType: .image) == nil) + } + + // MARK: - Preference order + + @Test func preferenceOrderIsScopedToTheAssetsMediaType() { + #expect(PhotoLibraryFileLoader.preferredResourceTypes(for: .image) == [.fullSizePhoto, .photo, .alternatePhoto]) + #expect(PhotoLibraryFileLoader.preferredResourceTypes(for: .video) == [.fullSizeVideo, .video]) + #expect(PhotoLibraryFileLoader.preferredResourceTypes(for: .audio) == [.audio]) + #expect(PhotoLibraryFileLoader.preferredResourceTypes(for: .unknown).isEmpty) + } +} + +/// Covers the switch that decides whether a picked item is read from the photo library or +/// from the item provider. +struct ItemProviderMediaExporterSourceTests { + + /// Outside Lockdown Mode the item provider stays the route even when the picker was + /// library-backed and supplied an asset identifier. + @Test func usesTheItemProviderWhenThePhotoLibrarySourceIsNotPreferred() async throws { + let exporter = try makeExporter(assetIdentifier: "any-identifier") + exporter.prefersPhotoLibrarySource = false + + let media = try await exporter.export() + + #expect(media.url.pathExtension == "jpeg") + MediaExporterTests.cleanUpExportedMedia(atURL: media.url) + } + + /// The asset can't be resolved, and the two routes are exclusive: retrying through + /// the item provider is pointless in Lockdown Mode — it's the thing that can't serve + /// the file — so the failure is reported instead of being silently papered over. + @Test func reportsAFailureRatherThanFallingBackWhenTheAssetCannotBeResolved() async throws { + let exporter = try makeExporter(assetIdentifier: "not-a-real-asset/L0/001") + exporter.prefersPhotoLibrarySource = true + + await #expect(throws: ItemProviderMediaExporter.ExportError.self) { + try await exporter.export() + } + } + + /// Without an asset identifier there is nothing to look up, so the item provider is + /// the only route even under Lockdown Mode. + @Test func usesTheItemProviderWhenThereIsNoAssetIdentifier() async throws { + let exporter = try makeExporter(assetIdentifier: nil) + exporter.prefersPhotoLibrarySource = true + + let media = try await exporter.export() + + #expect(media.url.pathExtension == "jpeg") + MediaExporterTests.cleanUpExportedMedia(atURL: media.url) + } + + private func makeExporter(assetIdentifier: String?) throws -> ItemProviderMediaExporter { + let imageURL = try #require(Bundle.test.url(forResource: "iphone-photo", withExtension: "heic")) + let provider = NSItemProvider() + provider.registerFileRepresentation(forTypeIdentifier: UTType.heic.identifier, visibility: .all) { completion in + completion(imageURL, false, nil) + return nil + } + let exporter = ItemProviderMediaExporter(provider: provider, assetIdentifier: assetIdentifier) + exporter.mediaDirectoryType = .temporary + return exporter + } +} diff --git a/WordPress/Classes/Services/MediaImportService.swift b/WordPress/Classes/Services/MediaImportService.swift index 83941b409f15..8502f5903019 100644 --- a/WordPress/Classes/Services/MediaImportService.swift +++ b/WordPress/Classes/Services/MediaImportService.swift @@ -328,6 +328,11 @@ class MediaImportService: NSObject { private func makeExporter(for exportable: ExportableAsset, options: ExportOptions) -> MediaExporter? { switch exportable { + case let item as PhotosPickerAsset: + let exporter = ItemProviderMediaExporter(provider: item.itemProvider, assetIdentifier: item.assetIdentifier) + exporter.imageOptions = options.imageOptions + exporter.videoOptions = options.videoOptions + return exporter case let provider as NSItemProvider: let exporter = ItemProviderMediaExporter(provider: provider) exporter.imageOptions = options.imageOptions diff --git a/WordPress/Classes/Utility/Media/ItemProviderMediaExporter.swift b/WordPress/Classes/Utility/Media/ItemProviderMediaExporter.swift index ab985e29bfa7..d744577235a5 100644 --- a/WordPress/Classes/Utility/Media/ItemProviderMediaExporter.swift +++ b/WordPress/Classes/Utility/Media/ItemProviderMediaExporter.swift @@ -9,10 +9,29 @@ final class ItemProviderMediaExporter: MediaExporter { var imageOptions: MediaImageExporter.Options? var videoOptions: MediaVideoExporter.Options? - private let provider: NSItemProvider + /// `nil` for an item picked with the legacy picker under Lockdown Mode, which vends + /// assets rather than providers — that route reads the file from the photo library. + private let provider: NSItemProvider? + private let assetIdentifier: String? - init(provider: NSItemProvider) { + /// Whether to read the file straight from the photo library instead of asking the + /// item provider for it. + /// + /// Defaults to Lockdown Mode, where the system's file provider extension can't + /// materialize large photos at all (see `handleLoadFailure` and + /// `PhotoLibraryFileLoader`). Everywhere else the provider is faster and needs no + /// Photos authorization, so it stays the default route. + /// + /// The two routes are exclusive: once this picks the photo library, a failure there + /// is reported rather than retried through the provider, which in Lockdown Mode is + /// the thing that doesn't work. + var prefersPhotoLibrarySource = LockdownHelper.isDeviceLockdownModeEnabled + + /// - parameter assetIdentifier: The local identifier of the `PHAsset` the item was + /// picked from, when the picker was library-backed. See `PhotosPickerAsset`. + init(provider: NSItemProvider?, assetIdentifier: String? = nil) { self.provider = provider + self.assetIdentifier = assetIdentifier } func export(onCompletion originalOnCompletion: @escaping (MediaExport) -> Void, onError originalOnError: @escaping (MediaExportError) -> Void) -> Progress { @@ -20,7 +39,8 @@ final class ItemProviderMediaExporter: MediaExporter { let onCompletion: (MediaExport) -> Void let onError: (MediaExportError) -> Void - // Create a temporary directory to hold the exported file from the `NSItemProvider` instance. + // Create a temporary directory to stage the picked file, whether it came from the + // `NSItemProvider` instance or straight from the photo library. let tempDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) do { try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) @@ -41,7 +61,7 @@ final class ItemProviderMediaExporter: MediaExporter { // It's important to use the `MediaImageExporter` because it strips the // GPS data and performs other image manipulations before the upload. - func processImage(at url: URL) throws { + func processImage(at url: URL, typeIdentifier: String?) throws { let exporter = MediaImageExporter(url: url) exporter.mediaDirectoryType = mediaDirectoryType if let imageOptions { @@ -49,7 +69,7 @@ final class ItemProviderMediaExporter: MediaExporter { } // If image format is not supported, switch to `.jpeg`. if exporter.options.exportImageType == nil, - let type = provider.registeredTypeIdentifiers.first, + let type = typeIdentifier, !ItemProviderMediaExporter.supportedImageTypes.contains(type) { exporter.options.exportImageType = UTType.jpeg.identifier } @@ -80,6 +100,66 @@ final class ItemProviderMediaExporter: MediaExporter { progress.addChild(exportProgress, withPendingUnitCount: MediaExportProgressUnits.halfDone) } + // The "process" functions are responsible for making sure the end result file + // (the one passed to `onCompletion` block) is located in the local Media library dir (`mediaFileManager`). + // + // `resourceTypeIdentifier` is the type of a file streamed from the photo library. + // It's `nil` for a file that came from the item provider, whose own registered + // types describe it instead. + func process(fileAt url: URL, resourceTypeIdentifier: String?) throws { + let resourceType = resourceTypeIdentifier.flatMap(UTType.init) + func hasType(_ type: UTType) -> Bool { + resourceType.map { $0.conforms(to: type) } ?? self.hasConformingType(type) + } + if hasType(.gif) { + try processGIF(at: url) + } else if hasType(.image) { + try processImage(at: url, typeIdentifier: resourceTypeIdentifier ?? self.provider?.registeredTypeIdentifiers.first) + } else if hasType(.movie) || hasType(.video) { + try processVideo(at: url) + } else { + onError(ExportError.unsupportedContentType) + } + } + + if prefersPhotoLibrarySource, let assetIdentifier { + let loadProgress = Progress.discreteProgress(totalUnitCount: MediaExportProgressUnits.done) + do { + // Retaining `self` on purpose. + try PhotoLibraryFileLoader.loadFile(assetIdentifier: assetIdentifier, into: tempDir, progress: loadProgress) { result in + switch result { + case .success(let file): + do { + try process(fileAt: file.url, resourceTypeIdentifier: file.typeIdentifier) + } catch { + onError(ExportError.underlyingError(error)) + } + case .failure(let error): + // A cancelled upload is reported by the upload coordinator, so + // surfacing it here would show a spurious failure. Clean up the + // partially streamed file that `onError` would have removed. + guard !loadProgress.isCancelled else { + try? FileManager.default.removeItem(at: tempDir) + return + } + self.handlePhotoLibraryFailure(error, onError: onError) + } + } + progress.addChild(loadProgress, withPendingUnitCount: MediaExportProgressUnits.halfDone) + } catch { + // No fallback: under Lockdown Mode the item provider is precisely what + // can't serve the file, so retrying through it would trade a clear + // failure for a silent one. + handlePhotoLibraryFailure(error, onError: onError) + } + return progress + } + + guard let provider else { + onError(ItemProviderMediaExporter.itemUnavailableError) + return progress + } + let start = CFAbsoluteTimeGetCurrent() DDLogInfo("Will export file for provider: \(ObjectIdentifier(provider)) \(provider.registeredTypeIdentifiers)") @@ -89,24 +169,13 @@ final class ItemProviderMediaExporter: MediaExporter { return } let diff = CFAbsoluteTimeGetCurrent() - start - DDLogInfo("Loaded file representation for provider: \(ObjectIdentifier(self.provider)) \(self.provider.registeredTypeIdentifiers) (\(diff) seconds)") + DDLogInfo("Loaded file representation for provider: \(ObjectIdentifier(provider)) \(provider.registeredTypeIdentifiers) (\(diff) seconds)") // Retaining `self` on purpose. do { let copyURL = tempDir.appendingPathComponent(url.lastPathComponent) try FileManager.default.copyItem(at: url, to: copyURL) - - // The "process" functions are responsible for making sure the end result file - // (the one passed to `onCompletion` block) is located in the local Media library dir (`mediaFileManager`). - if self.hasConformingType(.gif) { - try processGIF(at: copyURL) - } else if self.hasConformingType(.image) { - try processImage(at: copyURL) - } else if self.hasConformingType(.movie) || self.hasConformingType(.video) { - try processVideo(at: copyURL) - } else { - onError(ExportError.unsupportedContentType) - } + try process(fileAt: copyURL, resourceTypeIdentifier: nil) } catch { onError(ExportError.underlyingError(error)) } @@ -133,7 +202,7 @@ final class ItemProviderMediaExporter: MediaExporter { ].map(\.identifier)) private func hasConformingType(_ type: UTType) -> Bool { - provider.hasItemConformingToTypeIdentifier(type.identifier) + provider?.hasItemConformingToTypeIdentifier(type.identifier) ?? false } /// Surfaces a failure to load the picked file from the `NSItemProvider`. @@ -151,7 +220,7 @@ final class ItemProviderMediaExporter: MediaExporter { /// (e.g. 36 MP) fails and the `PhotosFileProvider` process is killed, giving /// `NSItemProviderError -1000` over `NSCocoaErrorDomain 4099`. private func handleLoadFailure(_ error: Error?, onError: (MediaExportError) -> Void) { - let providerID = ObjectIdentifier(provider) + let providerID = provider.map(ObjectIdentifier.init).map(String.init(describing:)) ?? "none" guard let error else { DDLogError("Failed to load file representation for provider: \(providerID), error: nil") onError(ExportError.unknown) @@ -167,34 +236,64 @@ final class ItemProviderMediaExporter: MediaExporter { } DDLogError("Failed to load file representation for provider: \(providerID), error: \(error)") if let connectionError = ItemProviderMediaExporter.providerConnectionError(in: error) { - let device = LockdownHelper.isDeviceLockdownModeEnabled - let appExcluded = device && !LockdownHelper.isAppLockdownModeEnabled - let properties = providerErrorProperties( - for: error, - connectionError: connectionError, - deviceLockdown: device, - appExcluded: appExcluded - ) + let properties = providerErrorProperties(for: error, connectionError: connectionError) WPAnalytics.track(.mediaImportItemUnavailable, properties: properties) - onError(device ? ExportError.lockdownModeRestricted : ExportError.cannotLoadItem) + onError(ItemProviderMediaExporter.itemUnavailableError) } else { onError(ExportError.underlyingError(error)) } } - private func providerErrorProperties(for error: Error, connectionError: NSError, deviceLockdown: Bool, appExcluded: Bool) -> [AnyHashable: Any] { + /// Surfaces a failure to read the picked file straight from the photo library — + /// either the asset couldn't be resolved or streaming it failed. + /// + /// That route is only taken under Lockdown Mode (see `PhotoLibraryFileLoader`), where + /// the item provider is precisely what can't serve the file — so there is nothing + /// left to fall back to and the failure is reported to the user. It's tracked under + /// the same event as a provider failure, told apart by `source`. + private func handlePhotoLibraryFailure(_ error: Error, onError: (MediaExportError) -> Void) { + DDLogError("Failed to read the picked asset from the photo library, error: \(error)") + let error = error as NSError + var properties = ItemProviderMediaExporter.lockdownProperties + properties["source"] = "photo_library" + properties["error_domain"] = error.domain + properties["error_code"] = error.code + properties["type_identifiers"] = typeIdentifiersDescription + WPAnalytics.track(.mediaImportItemUnavailable, properties: properties) + onError(ItemProviderMediaExporter.itemUnavailableError) + } + + private func providerErrorProperties(for error: Error, connectionError: NSError) -> [AnyHashable: Any] { let error = error as NSError + var properties = ItemProviderMediaExporter.lockdownProperties + properties["source"] = "item_provider" + properties["error_domain"] = error.domain + properties["error_code"] = error.code + properties["underlying_error_domain"] = connectionError.domain + properties["underlying_error_code"] = connectionError.code + properties["type_identifiers"] = typeIdentifiersDescription + return properties + } + + private var typeIdentifiersDescription: String { + provider?.registeredTypeIdentifiers.joined(separator: ", ") ?? "" + } + + /// The Lockdown Mode state recorded alongside an import failure. The device-wide flag + /// is the one that governs `PhotosFileProvider`; the per-app value is secondary. + private static var lockdownProperties: [AnyHashable: Any] { + let device = LockdownHelper.isDeviceLockdownModeEnabled return [ - "error_domain": error.domain, - "error_code": error.code, - "underlying_error_domain": connectionError.domain, - "underlying_error_code": connectionError.code, - "type_identifiers": provider.registeredTypeIdentifiers.joined(separator: ", "), - "lockdown_mode": deviceLockdown, - "lockdown_mode_app_excluded": appExcluded + "lockdown_mode": device, + "lockdown_mode_app_excluded": device && !LockdownHelper.isAppLockdownModeEnabled ] } + /// The message shown when the app can't get the picked file from either route. + private static var itemUnavailableError: ExportError { + LockdownHelper.isDeviceLockdownModeEnabled ? .lockdownModeRestricted : .cannotLoadItem + } + /// The XPC connection error codes (in `NSCocoaErrorDomain`) that signal the item /// provider's process died while producing the file. private static let xpcConnectionErrorCodes: Set = [ diff --git a/WordPress/Classes/Utility/Media/PhotoLibraryFileLoader.swift b/WordPress/Classes/Utility/Media/PhotoLibraryFileLoader.swift new file mode 100644 index 000000000000..016d140757c0 --- /dev/null +++ b/WordPress/Classes/Utility/Media/PhotoLibraryFileLoader.swift @@ -0,0 +1,195 @@ +import Foundation +import Photos +import UIKit +import UniformTypeIdentifiers + +/// Reads a picked photo-library asset directly from the library, bypassing the system's +/// `PhotosFileProvider` extension. +/// +/// The app normally gets picked media from the `NSItemProvider` vended by +/// `PHPickerViewController`, which is served by that extension. Under iOS Lockdown Mode +/// the extension performs a hardened full decode when it materializes an image and gets +/// killed at its 20 MB memory limit, so large photos can't be imported at all — a 36 MP +/// image needs roughly `36e6 × 4` bytes, or ~144 MB. The cost tracks megapixels, not file +/// size; videos stream without a decode and are unaffected in either mode. +/// +/// `PHAssetResourceManager` and `PHImageManager` are serviced by `photolibraryd`, which +/// has no such cap — the same 36 MP original streams to disk in tens of milliseconds. +/// They need Photos authorization and an asset identifier from a library-backed picker, +/// which is why the app only takes this route under Lockdown Mode. See +/// `PhotosPickerPresenter` for the picker side and `ItemProviderMediaExporter` for the +/// upload side. +enum PhotoLibraryFileLoader { + enum LoaderError: Error, CustomStringConvertible { + /// The picked asset couldn't be resolved in the library. + /// + /// Carries the app's Photos authorization, because that's usually the reason: + /// under `.limited`, an asset the picker let the user choose but that they never + /// granted the app access to doesn't resolve here. + case assetNotFound(authorization: PHAuthorizationStatus) + /// The asset has no resource attached to it. + case resourceNotFound + + var description: String { + switch self { + case .assetNotFound(let authorization): + return "assetNotFound(authorization: \(authorization.name))" + case .resourceNotFound: + return "resourceNotFound" + } + } + } + + /// A file streamed out of the photo library. + struct LoadedFile { + let url: URL + /// The type of the streamed resource, e.g. `public.heic`. Describes the file on + /// disk, which isn't necessarily the type the item provider would have produced. + let typeIdentifier: String + } + + // MARK: - Streaming the original file + + /// Streams the file backing `assetIdentifier` into `directory`, calling `completion` + /// on an arbitrary queue. + /// + /// Throws — without starting any work — when the asset or a resource to stream can't + /// be resolved, so the caller can fall back to the item provider. + /// + /// - parameter progress: Updated as the file streams. Cancelling it doesn't stop the + /// request (`PHAssetResourceManager` gives no way to cancel a write), but the + /// caller can use it to tell a cancellation apart from a genuine failure. + static func loadFile( + assetIdentifier: String, + into directory: URL, + progress: Progress, + completion: @escaping (Result) -> Void + ) throws { + guard let asset = fetchAsset(withIdentifier: assetIdentifier) else { + throw LoaderError.assetNotFound(authorization: PHPhotoLibrary.authorizationStatus(for: .readWrite)) + } + let resources = PHAssetResource.assetResources(for: asset) + guard let index = preferredResourceIndex(in: resources.map(\.type), mediaType: asset.mediaType) else { + throw LoaderError.resourceNotFound + } + let resource = resources[index] + let fileURL = directory.appendingPathComponent(filename(for: resource)) + + let options = PHAssetResourceRequestOptions() + options.isNetworkAccessAllowed = true // The original may still live in iCloud + options.progressHandler = { fraction in + progress.completedUnitCount = Int64(fraction * Double(progress.totalUnitCount)) + } + Loggers.app.info("Streaming picked asset \(resource.type.rawValue) resource from the photo library") + PHAssetResourceManager.default() + .writeData(for: resource, toFile: fileURL, options: options) { error in + if let error { + completion(.failure(error)) + } else { + progress.completedUnitCount = progress.totalUnitCount + completion(.success(LoadedFile(url: fileURL, typeIdentifier: resource.uniformTypeIdentifier))) + } + } + } + + /// The resource types to look for, most preferred first, for an asset of `mediaType`. + /// + /// The `fullSize` variants are the *current* rendition of an edited asset and only + /// exist once it has been edited; the plain variants are the untouched original. + /// Preferring the former uploads an edited photo with its edits, which is what the + /// picker produces with `preferredAssetRepresentationMode = .current`. + static func preferredResourceTypes(for mediaType: PHAssetMediaType) -> [PHAssetResourceType] { + switch mediaType { + case .image: return [.fullSizePhoto, .photo, .alternatePhoto] + case .video: return [.fullSizeVideo, .video] + case .audio: return [.audio] + default: return [] + } + } + + /// The index of the resource to stream, or `nil` when the asset has no resources. + /// + /// Matching on the asset's own media type keeps a Live Photo — which carries both a + /// `.photo` and a `.pairedVideo` resource — from uploading the wrong half. When none + /// of the preferred types are present the first resource is used, which is the best + /// guess available. + static func preferredResourceIndex(in types: [PHAssetResourceType], mediaType: PHAssetMediaType) -> Int? { + for preferred in preferredResourceTypes(for: mediaType) { + if let index = types.firstIndex(of: preferred) { + return index + } + } + return types.isEmpty ? nil : 0 + } + + /// A safe name for the streamed file. + /// + /// `originalFilename` is metadata carried by the library, so it's reduced to its last + /// path component to keep it from escaping the staging directory. + static func filename(for resource: PHAssetResource) -> String { + let filename = (resource.originalFilename as NSString).lastPathComponent + guard !filename.isEmpty, filename != ".", filename != ".." else { + let fileExtension = UTType(resource.uniformTypeIdentifier)?.preferredFilenameExtension + return fileExtension.map { "media.\($0)" } ?? "media" + } + return filename + } + + // MARK: - Loading a display image + + /// The largest image the app asks the library for. + /// + /// Everything on this path crops the result down to a site icon or an avatar, so a + /// full-resolution decode would be wasted work — and would reintroduce the memory + /// cost this type exists to avoid, just in the app instead of the extension. + private static let maximumImageSize = CGSize(width: 2048, height: 2048) + + /// Loads a display-ready image for `assetIdentifier`, or `nil` when the library can't + /// produce one. Calls `completion` on an arbitrary queue. + static func loadImage(assetIdentifier: String, completion: @escaping (UIImage?) -> Void) { + guard let asset = fetchAsset(withIdentifier: assetIdentifier) else { + return completion(nil) + } + let options = PHImageRequestOptions() + options.isNetworkAccessAllowed = true // The original may still live in iCloud + options.deliveryMode = .highQualityFormat + options.resizeMode = .exact + PHImageManager.default() + .requestImage( + for: asset, + targetSize: maximumImageSize, + contentMode: .aspectFit, + options: options + ) { image, _ in + // `.highQualityFormat` delivers a single, final result, so there's no + // degraded placeholder to filter out here. + completion(image) + } + } + + // MARK: - Helpers + + private static func fetchAsset(withIdentifier identifier: String) -> PHAsset? { + let options = PHFetchOptions() + // Left unset, the source types are inferred from the query, which for a fetch by + // local identifier means the user's own library only. The picker will happily + // hand over an asset that came from an iCloud Shared Album or an iTunes sync, so + // ask for all three rather than let those fall through to the item provider. + options.includeAssetSourceTypes = [.typeUserLibrary, .typeCloudShared, .typeiTunesSynced] + return PHAsset.fetchAssets(withLocalIdentifiers: [identifier], options: options).firstObject + } +} + +private extension PHAuthorizationStatus { + /// A readable name for logs; the raw values alone are hard to read back. + var name: String { + switch self { + case .notDetermined: return "notDetermined" + case .restricted: return "restricted" + case .denied: return "denied" + case .authorized: return "authorized" + case .limited: return "limited" + @unknown default: return "unknown(\(rawValue))" + } + } +} diff --git a/WordPress/Classes/ViewRelated/Aztec/ViewControllers/AztecPostViewController.swift b/WordPress/Classes/ViewRelated/Aztec/ViewControllers/AztecPostViewController.swift index d0d2b153d6a5..44e4c621f059 100644 --- a/WordPress/Classes/ViewRelated/Aztec/ViewControllers/AztecPostViewController.swift +++ b/WordPress/Classes/ViewRelated/Aztec/ViewControllers/AztecPostViewController.swift @@ -413,7 +413,7 @@ class AztecPostViewController: UIViewController, PostEditor { } private var mediaPickerInputViewController: PHPickerViewController? - private var selectedPickerResults: [PHPickerResult] = [] + private var selectedPickerAssets: [PhotosPickerAsset] = [] fileprivate var originalLeadingBarButtonGroup = [UIBarButtonItemGroup]() @@ -1919,6 +1919,10 @@ extension AztecPostViewController { richTextView.autocorrectionType = .no + // This picker is embedded as an input view with continuous selection, which only + // `PHPickerViewController` provides — so it stays as-is in Lockdown Mode, where + // large photos will still fail through the item provider. The full-screen "Device + // Photos" action is the route that gets the Lockdown-aware picker. var configuration = PHPickerConfiguration() configuration.filter = .any(of: [.images, .videos]) configuration.preferredAssetRepresentationMode = .current @@ -2424,9 +2428,10 @@ extension AztecPostViewController { /// Sets the badge title of `attachment` to "GIF". private func setGifBadgeIfNecessary(for attachment: MediaAttachment, asset: ExportableAsset) { - if let asset = (asset as? NSItemProvider), - asset.hasItemConformingToTypeIdentifier(UTType.gif.identifier) - { + // A picked Photos item carries its provider inside a `PhotosPickerAsset` rather + // than being one, so unwrap both shapes. + let provider = (asset as? NSItemProvider) ?? (asset as? PhotosPickerAsset)?.itemProvider + if provider?.hasItemConformingToTypeIdentifier(UTType.gif.identifier) == true { attachment.badgeTitle = Constants.mediaGIFBadgeTitle } } @@ -3141,7 +3146,7 @@ extension AztecPostViewController { } func closeMediaPickerInputViewController() { - selectedPickerResults = [] + selectedPickerAssets = [] mediaPickerInputViewController = nil changeRichTextInputView(to: nil) updateToolbar(formatBar, forMode: .text) @@ -3329,9 +3334,18 @@ extension AztecPostViewController: VideoLimitsAlertPresenter {} // MARK: - MediaPickerViewController (PHPickerViewControllerDelegate) +extension AztecPostViewController: DevicePhotosPickerDelegate { + /// The full-screen "Device Photos" action, which routes through `MediaPickerMenu` and + /// so gets the Lockdown-aware picker. + func devicePhotosPicker(didPick assets: [PhotosPickerAsset]) { + selectedPickerAssets = assets + insertPickerResults() + } +} + extension AztecPostViewController: PHPickerViewControllerDelegate { func picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult]) { - selectedPickerResults = results + selectedPickerAssets = results.map(PhotosPickerAsset.init) // The delegate is configured to get called continuously if picker == mediaPickerInputViewController { @@ -3343,17 +3357,17 @@ extension AztecPostViewController: PHPickerViewControllerDelegate { } private func insertPickerResults() { - guard !selectedPickerResults.isEmpty else { + guard !selectedPickerAssets.isEmpty else { return } - for result in selectedPickerResults { - insert(exportableAsset: result.itemProvider, source: .deviceLibrary) + for asset in selectedPickerAssets { + insert(exportableAsset: asset, source: .deviceLibrary) } closeMediaPickerInputViewController() } private func updateFormatBarInsertAssetCount() { - let assetCount = selectedPickerResults.count + let assetCount = selectedPickerAssets.count if assetCount == 0 { insertToolbarItem.isEnabled = false diff --git a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteIconPickerPresenter.swift b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteIconPickerPresenter.swift index f76b15edad89..597c7b2c4a48 100644 --- a/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteIconPickerPresenter.swift +++ b/WordPress/Classes/ViewRelated/Blog/Site Settings/SiteIconPickerPresenter.swift @@ -104,18 +104,17 @@ final class SiteIconPickerPresenter: NSObject { } } -extension SiteIconPickerPresenter: PHPickerViewControllerDelegate { - func picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult]) { - guard let result = results.first else { - picker.presentingViewController?.dismiss(animated: true) +extension SiteIconPickerPresenter: DevicePhotosPickerDelegate { + func devicePhotosPicker(didPick assets: [PhotosPickerAsset]) { + guard let asset = assets.first, let presentingViewController = UIViewController.topViewController else { return } WPAnalytics.track(.siteSettingsSiteIconGalleryPicked) self.showLoadingMessage() self.originalMedia = nil - PHPickerResult.loadImage(for: result) { [weak self] image, error in + asset.loadImage { [weak self] image, error in if let image { - self?.showImageCropViewController(image, presentingViewController: picker) + self?.showImageCropViewController(image, presentingViewController: presentingViewController) } else { DDLogError("Failed to load image: \(String(describing: error))") self?.showErrorLoadingImageMessage() diff --git a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergMediaInserterHelper.swift b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergMediaInserterHelper.swift index cf3d44d8939a..15a6856ef6e3 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergMediaInserterHelper.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergMediaInserterHelper.swift @@ -38,15 +38,15 @@ class GutenbergMediaInserterHelper: NSObject { } func insertFromDevice(_ selection: [Any], callback: @escaping MediaPickerDidPickMediaCallback) { - if let providers = selection as? [NSItemProvider] { - insertItemProviders(providers, callback: callback) + if let assets = selection as? [ExportableAsset] { + insertExportableAssets(assets, callback: callback) } else { callback(nil) } } - private func insertItemProviders(_ providers: [NSItemProvider], callback: @escaping MediaPickerDidPickMediaCallback) { - let media: [MediaInfo] = providers.compactMap { + private func insertExportableAssets(_ assets: [ExportableAsset], callback: @escaping MediaPickerDidPickMediaCallback) { + let media: [MediaInfo] = assets.compactMap { // WARNING: Media is a CoreData entity and has to be thread-confined guard let media = insert(exportableAsset: $0, source: .deviceLibrary) else { return nil diff --git a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergMediaPickerHelper.swift b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergMediaPickerHelper.swift index a9006e5e6321..2d2333d7bda0 100644 --- a/WordPress/Classes/ViewRelated/Gutenberg/GutenbergMediaPickerHelper.swift +++ b/WordPress/Classes/ViewRelated/Gutenberg/GutenbergMediaPickerHelper.swift @@ -44,17 +44,12 @@ final class GutenbergMediaPickerHelper: NSObject { ) { didPickMediaCallback = completion - var configuration = PHPickerConfiguration() - configuration.preferredAssetRepresentationMode = .current - if allowMultipleSelection { - configuration.selection = .ordered - configuration.selectionLimit = 0 - } - configuration.filter = PHPickerFilter(filter) - - let picker = PHPickerViewController(configuration: configuration) - picker.delegate = self - context.present(picker, animated: true) + PhotosPickerPresenter.present( + from: context, + filter: .init(filter), + isMultipleSelectionEnabled: allowMultipleSelection, + delegate: self + ) } func presentSiteMediaPicker( @@ -161,15 +156,12 @@ extension GutenbergMediaPickerHelper: SiteMediaPickerViewControllerDelegate { } } -extension GutenbergMediaPickerHelper: PHPickerViewControllerDelegate { - func picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult]) { - context.dismiss(animated: true) - - guard !results.isEmpty else { +extension GutenbergMediaPickerHelper: DevicePhotosPickerDelegate { + func devicePhotosPicker(didPick assets: [PhotosPickerAsset]) { + guard !assets.isEmpty else { return } - - didPickMediaCallback?(results.map(\.itemProvider)) + didPickMediaCallback?(assets) didPickMediaCallback = nil } } diff --git a/WordPress/Classes/ViewRelated/Me/My Profile/Gravatar/AvatarMenuController.swift b/WordPress/Classes/ViewRelated/Me/My Profile/Gravatar/AvatarMenuController.swift index 7b20b6b29b2c..e7e1110e31b5 100644 --- a/WordPress/Classes/ViewRelated/Me/My Profile/Gravatar/AvatarMenuController.swift +++ b/WordPress/Classes/ViewRelated/Me/My Profile/Gravatar/AvatarMenuController.swift @@ -3,7 +3,7 @@ import UIKit import PhotosUI import SVProgressHUD -final class AvatarMenuController: PHPickerViewControllerDelegate, ImagePickerControllerDelegate { +final class AvatarMenuController: DevicePhotosPickerDelegate, ImagePickerControllerDelegate { private weak var presentingViewController: UIViewController? var onAvatarSelected: ((UIImage) -> Void)? @@ -24,14 +24,13 @@ final class AvatarMenuController: PHPickerViewControllerDelegate, ImagePickerCon ]) } - // MARK: - PHPickerViewControllerDelegate + // MARK: - DevicePhotosPickerDelegate - func picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult]) { - guard let result = results.first else { - presentingViewController?.dismiss(animated: true) + func devicePhotosPicker(didPick assets: [PhotosPickerAsset]) { + guard let asset = assets.first else { return } - PHPickerResult.loadImage(for: result) { [weak self] image, _ in + asset.loadImage { [weak self] image, _ in if let image { self?.showCropViewController(with: image) } else { diff --git a/WordPress/Classes/ViewRelated/Media/MediaPicker/Helpers/MediaPickerController.swift b/WordPress/Classes/ViewRelated/Media/MediaPicker/Helpers/MediaPickerController.swift index 82acdb88f254..051f713cdd38 100644 --- a/WordPress/Classes/ViewRelated/Media/MediaPicker/Helpers/MediaPickerController.swift +++ b/WordPress/Classes/ViewRelated/Media/MediaPicker/Helpers/MediaPickerController.swift @@ -157,7 +157,7 @@ final class MediaPickerController: GutenbergKit.MediaPickerController { ) output.append(mediaInfo) - case .image, .pickerResult: + case .image, .deviceAsset: wpAssertionFailure("unused case") break } diff --git a/WordPress/Classes/ViewRelated/Media/MediaPicker/Helpers/MediaPickerMenuController.swift b/WordPress/Classes/ViewRelated/Media/MediaPicker/Helpers/MediaPickerMenuController.swift index ec379be630b9..2be7111d4d94 100644 --- a/WordPress/Classes/ViewRelated/Media/MediaPicker/Helpers/MediaPickerMenuController.swift +++ b/WordPress/Classes/ViewRelated/Media/MediaPicker/Helpers/MediaPickerMenuController.swift @@ -14,11 +14,10 @@ final class MediaPickerMenuController: NSObject { } } -extension MediaPickerMenuController: PHPickerViewControllerDelegate { - public func picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult]) { - picker.presentingViewController?.dismiss(animated: true) - if !results.isEmpty { - self.didSelect(results.map(MediaPickerItem.pickerResult), source: .applePhotos) +extension MediaPickerMenuController: DevicePhotosPickerDelegate { + func devicePhotosPicker(didPick assets: [PhotosPickerAsset]) { + if !assets.isEmpty { + self.didSelect(assets.map(MediaPickerItem.deviceAsset), source: .applePhotos) } } } diff --git a/WordPress/Classes/ViewRelated/Media/MediaPicker/MediaPicker.swift b/WordPress/Classes/ViewRelated/Media/MediaPicker/MediaPicker.swift index b8151fcdf81c..00898aa2cf44 100644 --- a/WordPress/Classes/ViewRelated/Media/MediaPicker/MediaPicker.swift +++ b/WordPress/Classes/ViewRelated/Media/MediaPicker/MediaPicker.swift @@ -101,7 +101,7 @@ struct MediaPickerSelection { } enum MediaPickerItem { - case pickerResult(PHPickerResult) + case deviceAsset(PhotosPickerAsset) case image(UIImage) case media(Media) case external(ExternalMediaAsset) @@ -110,8 +110,8 @@ enum MediaPickerItem { /// is already uploaded, returns `Media`. func exported() -> Exportable { switch self { - case .pickerResult(let result): - return .asset(result.itemProvider) + case .deviceAsset(let asset): + return .asset(asset) case .image(let image): return .asset(image) case .media(let media): diff --git a/WordPress/Classes/ViewRelated/Media/MediaPicker/Menu/MediaPickerMenu+Photos.swift b/WordPress/Classes/ViewRelated/Media/MediaPicker/Menu/MediaPickerMenu+Photos.swift index 243851fcd756..724d93e0f14d 100644 --- a/WordPress/Classes/ViewRelated/Media/MediaPicker/Menu/MediaPickerMenu+Photos.swift +++ b/WordPress/Classes/ViewRelated/Media/MediaPicker/Menu/MediaPickerMenu+Photos.swift @@ -1,11 +1,12 @@ import UIKit +import Photos import PhotosUI extension MediaPickerMenu { /// Returns an action for picking photos from the device's Photos library. /// - /// - note: Use `PHPickerResult.loadImage(for:)` to retrieve an image from the result. - func makePhotosAction(delegate: PHPickerViewControllerDelegate) -> UIAction { + /// - note: Use `PhotosPickerAsset.loadImage(_:)` to retrieve an image from the result. + func makePhotosAction(delegate: DevicePhotosPickerDelegate) -> UIAction { UIAction( title: Strings.pickFromPhotosLibrary, image: UIImage(systemName: "photo.on.rectangle.angled"), @@ -14,24 +15,14 @@ extension MediaPickerMenu { ) } - func showPhotosPicker(delegate: PHPickerViewControllerDelegate) { - var configuration = PHPickerConfiguration() - configuration.preferredAssetRepresentationMode = .current - if let filter { - switch filter { - case .images: - configuration.filter = .images - case .videos: - configuration.filter = .videos - } - } - if isMultipleSelectionEnabled { - configuration.selectionLimit = 0 - configuration.selection = .ordered - } - let picker = PHPickerViewController(configuration: configuration) - picker.delegate = delegate - presentingViewController?.present(picker, animated: true) + func showPhotosPicker(delegate: DevicePhotosPickerDelegate) { + guard let presentingViewController else { return } + PhotosPickerPresenter.present( + from: presentingViewController, + filter: filter, + isMultipleSelectionEnabled: isMultipleSelectionEnabled, + delegate: delegate + ) } } diff --git a/WordPress/Classes/ViewRelated/Media/PHPickerController+Extensions.swift b/WordPress/Classes/ViewRelated/Media/PHPickerController+Extensions.swift index a07567be0e54..3cfcb22fc193 100644 --- a/WordPress/Classes/ViewRelated/Media/PHPickerController+Extensions.swift +++ b/WordPress/Classes/ViewRelated/Media/PHPickerController+Extensions.swift @@ -21,15 +21,6 @@ extension PHPickerFilter { } } -extension PHPickerResult { - /// Retrieves an image for the given picker result. - /// - /// - parameter completion: The completion closure that gets called on the main thread. - static func loadImage(for result: PHPickerResult, _ completion: @escaping (UIImage?, Error?) -> Void) { - NSItemProvider.loadImage(for: result.itemProvider, completion) - } -} - extension NSItemProvider { // MARK: - Images diff --git a/WordPress/Classes/ViewRelated/Media/PhotosPickerAsset.swift b/WordPress/Classes/ViewRelated/Media/PhotosPickerAsset.swift new file mode 100644 index 000000000000..3076f22bd5a2 --- /dev/null +++ b/WordPress/Classes/ViewRelated/Media/PhotosPickerAsset.swift @@ -0,0 +1,86 @@ +import Foundation +import Photos +import PhotosUI +import UIKit +import WordPressData + +/// A media item picked from the device's Photos library. +/// +/// The two pickers the app presents hand back different things, and this is what they +/// have in common. Outside Lockdown Mode `PHPickerViewController` gives an +/// `NSItemProvider`; under Lockdown Mode the legacy picker gives a `PHAsset`, which is +/// carried here as its local identifier so `ItemProviderMediaExporter` can read the file +/// straight from the library. See `PhotosPickerPresenter` for why they differ. +final class PhotosPickerAsset: NSObject, ExportableAsset { + /// The picker's item provider. `nil` for an item picked with the legacy picker, which + /// vends assets rather than providers. + let itemProvider: NSItemProvider? + + /// The local identifier of the backing `PHAsset`, when the picker supplied one. + let assetIdentifier: String? + + let assetMediaType: MediaType + + init(itemProvider: NSItemProvider?, assetIdentifier: String?, assetMediaType: MediaType) { + self.itemProvider = itemProvider + self.assetIdentifier = assetIdentifier + self.assetMediaType = assetMediaType + } + + convenience init(_ result: PHPickerResult) { + self.init( + itemProvider: result.itemProvider, + assetIdentifier: result.assetIdentifier, + assetMediaType: result.itemProvider.assetMediaType + ) + } + + convenience init(_ asset: PHAsset) { + self.init( + itemProvider: nil, + assetIdentifier: asset.localIdentifier, + assetMediaType: MediaType(asset.mediaType) + ) + } +} + +extension PhotosPickerAsset { + /// Retrieves an image for the item, for the flows that crop one rather than upload it. + /// + /// Prefers the photo library when the item came from there, both because it's the only + /// source the legacy picker gives us and because the item provider can't materialize a + /// large photo under Lockdown Mode — see `PhotoLibraryFileLoader`. + /// + /// - parameter completion: Called on the main thread. + func loadImage(_ completion: @escaping (UIImage?, Error?) -> Void) { + guard let assetIdentifier, itemProvider == nil || LockdownHelper.isDeviceLockdownModeEnabled else { + return loadImageFromItemProvider(completion) + } + PhotoLibraryFileLoader.loadImage(assetIdentifier: assetIdentifier) { [self] image in + guard let image else { + return loadImageFromItemProvider(completion) + } + DispatchQueue.main.async { + completion(image, nil) + } + } + } + + private func loadImageFromItemProvider(_ completion: @escaping (UIImage?, Error?) -> Void) { + guard let itemProvider else { + return DispatchQueue.main.async { completion(nil, nil) } + } + NSItemProvider.loadImage(for: itemProvider, completion) + } +} + +private extension MediaType { + init(_ mediaType: PHAssetMediaType) { + switch mediaType { + case .image: self = .image + case .video: self = .video + case .audio: self = .audio + default: self = .document + } + } +} diff --git a/WordPress/Classes/ViewRelated/Media/PhotosPickerPresenter.swift b/WordPress/Classes/ViewRelated/Media/PhotosPickerPresenter.swift new file mode 100644 index 000000000000..68e3bf185b28 --- /dev/null +++ b/WordPress/Classes/ViewRelated/Media/PhotosPickerPresenter.swift @@ -0,0 +1,175 @@ +import Photos +import PhotosUI +import UIKit +import WordPressShared + +/// Receives the result of picking media from the device's Photos library. +protocol DevicePhotosPickerDelegate: AnyObject { + /// - parameter assets: Empty if the user cancelled. + func devicePhotosPicker(didPick assets: [PhotosPickerAsset]) +} + +/// Presents a picker for the device's Photos library, choosing the one that works in the +/// current Lockdown Mode state. +/// +/// Two pickers, because they answer different questions: +/// +/// - **Outside Lockdown Mode**, `PHPickerViewController` runs out-of-process, needs no +/// Photos authorization, and hands back an `NSItemProvider`. That's the best experience +/// and it's what the app has always used. +/// - **Under Lockdown Mode**, the file has to be read through `PHAssetResourceManager` +/// (see `PhotoLibraryFileLoader`), which only works for assets the app can actually +/// resolve. `PHPickerViewController` is the wrong tool for that: even when it's backed +/// by the shared library it displays the *whole* library regardless of what the app has +/// been granted, so under limited authorization it offers items that then can't be read. +/// +/// No system picker shows only the granted assets. The legacy `UIImagePickerController` +/// is not an escape hatch: it runs out-of-process too (it presents +/// `_UIImagePickerPlaceholderViewController`, a remote view controller host), and since +/// limited authorization was introduced in iOS 14 it leaves +/// `UIImagePickerController.InfoKey.phAsset` nil whenever access is `.limited` — even for +/// an asset the app *is* allowed to read. So it can only supply an asset under full +/// authorization, which is precisely the case where this picker already works. Scoping a +/// grid to what the app can read would mean building one on `PHAsset.fetchAssets`. +enum PhotosPickerPresenter { + /// Presents the appropriate picker from `viewController`. + static func present( + from viewController: UIViewController, + filter: MediaPickerMenu.MediaFilter?, + isMultipleSelectionEnabled: Bool, + delegate: DevicePhotosPickerDelegate + ) { + guard LockdownHelper.isDeviceLockdownModeEnabled else { + presentSystemPicker( + from: viewController, + filter: filter, + isMultipleSelectionEnabled: isMultipleSelectionEnabled, + delegate: delegate + ) + return + } + PHPhotoLibrary.requestAuthorization(for: .readWrite) { [weak viewController, weak delegate] status in + DispatchQueue.main.async { + guard let viewController, let delegate else { return } + switch status { + case .authorized: + presentSystemPicker( + from: viewController, + filter: filter, + isMultipleSelectionEnabled: isMultipleSelectionEnabled, + delegate: delegate, + photoLibrary: .shared() + ) + case .limited: + // Limited access isn't enough here: the picker shows the whole library + // whatever the app has been granted, so letting it open would mostly + // offer items that can't then be read. Ask for full access instead. + showFullAccessRequiredAlert(from: viewController) + default: + // Without authorization there's no library to read from, so fall back + // to the permission-free picker. A file too large to materialize then + // surfaces a Lockdown-specific error rather than hanging. + presentSystemPicker( + from: viewController, + filter: filter, + isMultipleSelectionEnabled: isMultipleSelectionEnabled, + delegate: delegate + ) + } + } + } + } + + // MARK: - PHPickerViewController + + private static func presentSystemPicker( + from viewController: UIViewController, + filter: MediaPickerMenu.MediaFilter?, + isMultipleSelectionEnabled: Bool, + delegate: DevicePhotosPickerDelegate, + photoLibrary: PHPhotoLibrary? = nil + ) { + // Backed by the library only under Lockdown Mode: that's what makes results carry + // an `assetIdentifier`, which is what `PhotoLibraryFileLoader` needs. + var configuration = photoLibrary.map(PHPickerConfiguration.init) ?? PHPickerConfiguration() + configuration.preferredAssetRepresentationMode = .current + if let filter { + switch filter { + case .images: configuration.filter = .images + case .videos: configuration.filter = .videos + } + } + if isMultipleSelectionEnabled { + configuration.selectionLimit = 0 + configuration.selection = .ordered + } + let picker = PHPickerViewController(configuration: configuration) + let adapter = SystemPickerAdapter(delegate: delegate) + picker.delegate = adapter + retain(adapter, on: picker) + viewController.present(picker, animated: true) + } + + private final class SystemPickerAdapter: NSObject, PHPickerViewControllerDelegate { + private weak var delegate: DevicePhotosPickerDelegate? + + init(delegate: DevicePhotosPickerDelegate) { + self.delegate = delegate + } + + func picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult]) { + picker.presentingViewController?.dismiss(animated: true) + delegate?.devicePhotosPicker(didPick: results.map(PhotosPickerAsset.init)) + } + } + + // MARK: - Limited access + + /// Explains that full access is required under Lockdown Mode and offers to open + /// Settings. There's no "continue anyway": with limited access the picker would offer + /// items the app can't read, which is the failure this whole path exists to avoid. + private static func showFullAccessRequiredAlert(from viewController: UIViewController) { + let alert = UIAlertController( + title: Strings.limitedAccessTitle, + message: Strings.limitedAccessMessage, + preferredStyle: .alert + ) + let openSettings = UIAlertAction(title: Strings.openSettings, style: .default) { _ in + guard let url = URL(string: UIApplication.openSettingsURLString) else { + return wpAssertionFailure("Failed to create the Open Settings URL") + } + UIApplication.shared.open(url) + } + alert.addAction(openSettings) + alert.addAction(UIAlertAction(title: SharedStrings.Button.cancel, style: .cancel)) + alert.preferredAction = openSettings + viewController.present(alert, animated: true) + } + + // MARK: - Helpers + + /// The picker holds its delegate weakly, so the adapter has to live on the picker. + private static func retain(_ adapter: NSObject, on picker: UIViewController) { + objc_setAssociatedObject(picker, &adapterKey, adapter, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) + } + + private nonisolated(unsafe) static var adapterKey: UInt8 = 0 +} + +private enum Strings { + static let limitedAccessTitle = NSLocalizedString( + "mediaPicker.limitedAccess.title", + value: "Allow Full Access", + comment: "Title of an alert shown when the app only has access to some photos and needs Full Access" + ) + static let limitedAccessMessage = NSLocalizedString( + "mediaPicker.limitedAccess.message", + value: "While Lockdown Mode is on, adding media requires Full Access enabled. You can change this in Settings.", + comment: "Message of an alert shown when the app only has access to some photos and needs Full Access. \"Full Access\" matches the option name in the iOS Settings app." + ) + static let openSettings = NSLocalizedString( + "mediaPicker.limitedAccess.openSettings", + value: "Open Settings", + comment: "Button that opens the Settings app" + ) +} diff --git a/WordPress/Classes/ViewRelated/Media/SiteMedia/Controllers/SiteMediaAddMediaMenuController.swift b/WordPress/Classes/ViewRelated/Media/SiteMedia/Controllers/SiteMediaAddMediaMenuController.swift index 518bc2af9fc9..a767b0ccb8cb 100644 --- a/WordPress/Classes/ViewRelated/Media/SiteMedia/Controllers/SiteMediaAddMediaMenuController.swift +++ b/WordPress/Classes/ViewRelated/Media/SiteMedia/Controllers/SiteMediaAddMediaMenuController.swift @@ -5,7 +5,7 @@ import PhotosUI import WordPressData import WordPressShared -final class SiteMediaAddMediaMenuController: NSObject, PHPickerViewControllerDelegate, ImagePickerControllerDelegate, +final class SiteMediaAddMediaMenuController: NSObject, DevicePhotosPickerDelegate, ImagePickerControllerDelegate, ExternalMediaPickerViewDelegate, UIDocumentPickerDelegate, ImagePlaygroundPickerDelegate { // swiftlint:disable:this opening_brace let blog: Blog @@ -70,18 +70,12 @@ final class SiteMediaAddMediaMenuController: NSObject, PHPickerViewControllerDel viewController.present(UIHostingController(rootView: rootView), animated: true) } - // MARK: - PHPickerViewControllerDelegate + // MARK: - DevicePhotosPickerDelegate - func picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult]) { - picker.presentingViewController?.dismiss(animated: true) - - guard !results.isEmpty else { - return - } - - for result in results { + func devicePhotosPicker(didPick assets: [PhotosPickerAsset]) { + for asset in assets { let info = MediaAnalyticsInfo(origin: .mediaLibrary(.deviceLibrary), selectionMethod: .fullScreenPicker) - coordinator.addMedia(from: result.itemProvider, to: blog, analyticsInfo: info) + coordinator.addMedia(from: asset, to: blog, analyticsInfo: info) } } From 12ab67887436446fdd1510f735dbbdbeeeb96549 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Tue, 8 Sep 2026 15:40:39 -0600 Subject: [PATCH 2/2] Require full Photos access under Lockdown Mode for any non-full state Denied access previously fell through to the permission-free picker while limited access was refused outright, which inverted user intent: granting *some* access left you worse off than granting none. Both now get the same answer. --- .../Media/PhotosPickerPresenter.swift | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/WordPress/Classes/ViewRelated/Media/PhotosPickerPresenter.swift b/WordPress/Classes/ViewRelated/Media/PhotosPickerPresenter.swift index 68e3bf185b28..bb931f5b61b0 100644 --- a/WordPress/Classes/ViewRelated/Media/PhotosPickerPresenter.swift +++ b/WordPress/Classes/ViewRelated/Media/PhotosPickerPresenter.swift @@ -60,21 +60,13 @@ enum PhotosPickerPresenter { delegate: delegate, photoLibrary: .shared() ) - case .limited: - // Limited access isn't enough here: the picker shows the whole library - // whatever the app has been granted, so letting it open would mostly - // offer items that can't then be read. Ask for full access instead. - showFullAccessRequiredAlert(from: viewController) default: - // Without authorization there's no library to read from, so fall back - // to the permission-free picker. A file too large to materialize then - // surfaces a Lockdown-specific error rather than hanging. - presentSystemPicker( - from: viewController, - filter: filter, - isMultipleSelectionEnabled: isMultipleSelectionEnabled, - delegate: delegate - ) + // Anything short of full access can't work here. Limited access is no + // help because the picker shows the whole library whatever the app has + // been granted, so it would offer items that can't then be read; with + // no access there's no library to read from at all. Both get the same + // answer, so that granting *some* access isn't worse than granting none. + showFullAccessRequiredAlert(from: viewController) } } } @@ -126,8 +118,8 @@ enum PhotosPickerPresenter { // MARK: - Limited access /// Explains that full access is required under Lockdown Mode and offers to open - /// Settings. There's no "continue anyway": with limited access the picker would offer - /// items the app can't read, which is the failure this whole path exists to avoid. + /// Settings. There's no "continue anyway": the picker would offer items the app can't + /// read, which is the failure this whole path exists to avoid. private static func showFullAccessRequiredAlert(from viewController: UIViewController) { let alert = UIAlertController( title: Strings.limitedAccessTitle,