From d962a3d9c1468bbb92ca41a8739d625e9cfdfb44 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 30 Jul 2026 09:09:35 -0700 Subject: [PATCH] fix(ios): avoid duplicate URLs in shared drafts (#116430) * fix(ios): deduplicate shared URLs * test(ios): prove share provider extraction --- .../ShareContentExtractor.swift | 142 ++++++++++++++++ .../ShareExtension/ShareDraftComposer.swift | 5 + .../ShareExtension/ShareViewController.swift | 152 ++---------------- .../Logic/ShareContentExtractorTests.swift | 76 +++++++++ .../Tests/Logic/ShareDraftComposerTests.swift | 31 ++++ apps/ios/project.yml | 2 + 6 files changed, 270 insertions(+), 138 deletions(-) create mode 100644 apps/ios/ShareExtension/ShareContentExtractor.swift create mode 100644 apps/ios/Tests/Logic/ShareContentExtractorTests.swift diff --git a/apps/ios/ShareExtension/ShareContentExtractor.swift b/apps/ios/ShareExtension/ShareContentExtractor.swift new file mode 100644 index 000000000000..c7f4192a108c --- /dev/null +++ b/apps/ios/ShareExtension/ShareContentExtractor.swift @@ -0,0 +1,142 @@ +import Foundation +import OpenClawKit +import UniformTypeIdentifiers + +struct ExtractedShareProviderContent { + var payload: SharedContentPayload + var imageProviders: [NSItemProvider] + var attachmentSummary: ShareAttachmentSummary +} + +@MainActor +enum ShareContentExtractor { + static func extract(from items: [NSExtensionItem]) async -> ExtractedShareProviderContent { + var title: String? + var attributedContentText: String? + var providers: [NSItemProvider] = [] + for item in items { + if title == nil { title = item.attributedTitle?.string } + if attributedContentText == nil { attributedContentText = item.attributedContentText?.string } + providers.append(contentsOf: item.attachments ?? []) + } + + var providersWithLoadedContent = Set() + var sharedURL: URL? + for provider in providers { + guard let providerURL = await self.loadURL(from: provider) else { continue } + sharedURL = providerURL + providersWithLoadedContent.insert(ObjectIdentifier(provider)) + break + } + + var sharedText: String? + for provider in providers { + guard let providerText = await self.loadText(from: provider) else { continue } + providersWithLoadedContent.insert(ObjectIdentifier(provider)) + guard let distinctText = SharePayloadNormalizer.distinctProviderText( + providerText, + sharedURL: sharedURL) + else { continue } + sharedText = distinctText + break + } + + var imageProviders: [NSItemProvider] = [] + var attachmentSummary = ShareAttachmentSummary() + for provider in providers { + if provider.hasItemConformingToTypeIdentifier(UTType.image.identifier) { + attachmentSummary.selectedImageCount += 1 + imageProviders.append(provider) + } else if provider.hasItemConformingToTypeIdentifier(UTType.movie.identifier) { + attachmentSummary.videoCount += 1 + } else if provider.hasItemConformingToTypeIdentifier(UTType.fileURL.identifier) { + attachmentSummary.fileCount += 1 + } else { + // UTI conformance only promises a representation exists; count it as handled + // only after the provider successfully delivers content we can send. + attachmentSummary.recordUnclassifiedProvider( + didLoadContent: providersWithLoadedContent.contains(ObjectIdentifier(provider))) + } + } + + let supplementalTitle = SharePayloadNormalizer.distinctAttributedText( + attributedContentText, + sharedText: sharedText, + sharedURL: sharedURL) + return ExtractedShareProviderContent( + payload: SharedContentPayload(title: title ?? supplementalTitle, url: sharedURL, text: sharedText), + imageProviders: imageProviders, + attachmentSummary: attachmentSummary) + } + + private static func loadURL(from provider: NSItemProvider) async -> URL? { + if provider.hasItemConformingToTypeIdentifier(UTType.url.identifier), + let url = await self.loadURLValue(from: provider, typeIdentifier: UTType.url.identifier) + { + return url + } + + if provider.hasItemConformingToTypeIdentifier(UTType.text.identifier), + let text = await self.loadTextValue(from: provider, typeIdentifier: UTType.text.identifier), + let url = SharePayloadNormalizer.webURL(from: text) + { + return url + } + + return nil + } + + private static func loadText(from provider: NSItemProvider) async -> String? { + if provider.hasItemConformingToTypeIdentifier(UTType.plainText.identifier), + let text = await self.loadTextValue(from: provider, typeIdentifier: UTType.plainText.identifier) + { + return text + } + + if provider.hasItemConformingToTypeIdentifier(UTType.text.identifier), + let text = await self.loadTextValue(from: provider, typeIdentifier: UTType.text.identifier) + { + return text + } + + if provider.hasItemConformingToTypeIdentifier(UTType.url.identifier), + let url = await self.loadURLValue(from: provider, typeIdentifier: UTType.url.identifier) + { + return url.absoluteString + } + + return nil + } + + private static func loadURLValue(from provider: NSItemProvider, typeIdentifier: String) async -> URL? { + await withCheckedContinuation { continuation in + provider.loadItem(forTypeIdentifier: typeIdentifier, options: nil) { item, _ in + if let url = item as? URL { + continuation.resume(returning: url) + } else if let value = item as? String, let url = URL(string: value) { + continuation.resume(returning: url) + } else if let value = item as? NSString, let url = URL(string: value as String) { + continuation.resume(returning: url) + } else { + continuation.resume(returning: nil) + } + } + } + } + + private static func loadTextValue(from provider: NSItemProvider, typeIdentifier: String) async -> String? { + await withCheckedContinuation { continuation in + provider.loadItem(forTypeIdentifier: typeIdentifier, options: nil) { item, _ in + if let text = item as? String { + continuation.resume(returning: text) + } else if let text = item as? NSString { + continuation.resume(returning: text as String) + } else if let text = item as? NSAttributedString { + continuation.resume(returning: text.string) + } else { + continuation.resume(returning: nil) + } + } + } + } +} diff --git a/apps/ios/ShareExtension/ShareDraftComposer.swift b/apps/ios/ShareExtension/ShareDraftComposer.swift index 7f781357276a..ca09c0b659c8 100644 --- a/apps/ios/ShareExtension/ShareDraftComposer.swift +++ b/apps/ios/ShareExtension/ShareDraftComposer.swift @@ -38,6 +38,11 @@ enum ShareDraftComposer { } enum SharePayloadNormalizer { + static func distinctProviderText(_ raw: String?, sharedURL: URL?) -> String? { + guard let candidate = self.trimmed(raw) else { return nil } + return candidate == self.trimmed(sharedURL?.absoluteString) ? nil : candidate + } + static func distinctAttributedText(_ raw: String?, sharedText: String?, sharedURL: URL?) -> String? { guard let candidate = self.trimmed(raw) else { return nil } let duplicates = [self.trimmed(sharedText), self.trimmed(sharedURL?.absoluteString)].compactMap(\.self) diff --git a/apps/ios/ShareExtension/ShareViewController.swift b/apps/ios/ShareExtension/ShareViewController.swift index 0b08ffad38f5..27c16d870182 100644 --- a/apps/ios/ShareExtension/ShareViewController.swift +++ b/apps/ios/ShareExtension/ShareViewController.swift @@ -289,69 +289,28 @@ final class ShareViewController: UIViewController { attachmentError: nil) } - var title: String? - var sharedURL: URL? - var sharedText: String? - var attributedContentText: String? + let providerContent = await ShareContentExtractor.extract(from: items) var attachments: [LoadedAttachment] = [] - var attachmentSummary = ShareAttachmentSummary() + var attachmentSummary = providerContent.attachmentSummary var attachmentError: ShareImageProcessor.ProcessError? let maxImageAttachments = 3 - for item in items { - if title == nil { - title = item.attributedTitle?.string - } - if attributedContentText == nil { - attributedContentText = item.attributedContentText?.string - } - - for provider in item.attachments ?? [] { - let providerURL = sharedURL == nil ? await self.loadURL(from: provider) : nil - let providerText = sharedText == nil ? await self.loadText(from: provider) : nil - if let providerURL { - sharedURL = providerURL - } - if let providerText { - sharedText = providerText - } - - if provider.hasItemConformingToTypeIdentifier(UTType.image.identifier) { - attachmentSummary.selectedImageCount += 1 - if attachments.count < maxImageAttachments, attachmentError == nil { - do { - let attachment = try await self.loadImageAttachment( - from: provider, - index: attachments.count) - attachments.append(attachment) - } catch let error as ShareImageProcessor.ProcessError { - attachmentError = error - } catch { - attachmentError = .encodeFailed - } - } - } else if provider.hasItemConformingToTypeIdentifier(UTType.movie.identifier) { - attachmentSummary.videoCount += 1 - } else if provider.hasItemConformingToTypeIdentifier(UTType.fileURL.identifier) { - attachmentSummary.fileCount += 1 - } else { - // UTI conformance only promises a representation exists; count it as handled - // only after the provider successfully delivers content we can send. - attachmentSummary.recordUnclassifiedProvider( - didLoadContent: providerURL != nil || providerText != nil) - } + for provider in providerContent.imageProviders.prefix(maxImageAttachments) { + guard attachmentError == nil else { break } + do { + let attachment = try await self.loadImageAttachment( + from: provider, + index: attachments.count) + attachments.append(attachment) + } catch let error as ShareImageProcessor.ProcessError { + attachmentError = error + } catch { + attachmentError = .encodeFailed } } attachmentSummary.acceptedImageCount = attachments.count - - // Share hosts often mirror provider text in attributedContentText. - // Preserve distinct content as the historical title, but do not duplicate provider data. - let supplementalTitle = SharePayloadNormalizer.distinctAttributedText( - attributedContentText, - sharedText: sharedText, - sharedURL: sharedURL) return ExtractedShareContent( - payload: SharedContentPayload(title: title ?? supplementalTitle, url: sharedURL, text: sharedText), + payload: providerContent.payload, attachments: attachments, attachmentSummary: attachmentSummary, attachmentError: attachmentError) @@ -426,89 +385,6 @@ final class ShareViewController: UIViewController { return nil } - private func loadURL(from provider: NSItemProvider) async -> URL? { - if provider.hasItemConformingToTypeIdentifier(UTType.url.identifier) { - if let url = await self.loadURLValue( - from: provider, - typeIdentifier: UTType.url.identifier) - { - return url - } - } - - if provider.hasItemConformingToTypeIdentifier(UTType.text.identifier) { - if let text = await self.loadTextValue(from: provider, typeIdentifier: UTType.text.identifier), - let url = SharePayloadNormalizer.webURL(from: text) - { - return url - } - } - - return nil - } - - private func loadText(from provider: NSItemProvider) async -> String? { - if provider.hasItemConformingToTypeIdentifier(UTType.plainText.identifier) { - if let text = await self.loadTextValue(from: provider, typeIdentifier: UTType.plainText.identifier) { - return text - } - } - - if provider.hasItemConformingToTypeIdentifier(UTType.text.identifier) { - if let text = await self.loadTextValue(from: provider, typeIdentifier: UTType.text.identifier) { - return text - } - } - - if provider.hasItemConformingToTypeIdentifier(UTType.url.identifier) { - if let url = await self.loadURLValue(from: provider, typeIdentifier: UTType.url.identifier) { - return url.absoluteString - } - } - - return nil - } - - private func loadURLValue(from provider: NSItemProvider, typeIdentifier: String) async -> URL? { - await withCheckedContinuation { continuation in - provider.loadItem(forTypeIdentifier: typeIdentifier, options: nil) { item, _ in - if let url = item as? URL { - continuation.resume(returning: url) - return - } - if let str = item as? String, let url = URL(string: str) { - continuation.resume(returning: url) - return - } - if let ns = item as? NSString, let url = URL(string: ns as String) { - continuation.resume(returning: url) - return - } - continuation.resume(returning: nil) - } - } - } - - private func loadTextValue(from provider: NSItemProvider, typeIdentifier: String) async -> String? { - await withCheckedContinuation { continuation in - provider.loadItem(forTypeIdentifier: typeIdentifier, options: nil) { item, _ in - if let text = item as? String { - continuation.resume(returning: text) - return - } - if let text = item as? NSString { - continuation.resume(returning: text as String) - return - } - if let text = item as? NSAttributedString { - continuation.resume(returning: text.string) - return - } - continuation.resume(returning: nil) - } - } - } - private func loadDataValue(from provider: NSItemProvider, typeIdentifier: String) async -> Data? { await withCheckedContinuation { continuation in provider.loadDataRepresentation(forTypeIdentifier: typeIdentifier) { data, _ in diff --git a/apps/ios/Tests/Logic/ShareContentExtractorTests.swift b/apps/ios/Tests/Logic/ShareContentExtractorTests.swift new file mode 100644 index 000000000000..cec2d02d88d2 --- /dev/null +++ b/apps/ios/Tests/Logic/ShareContentExtractorTests.swift @@ -0,0 +1,76 @@ +import Foundation +import Testing +import UniformTypeIdentifiers + +struct ShareContentExtractorTests { + @Test @MainActor func `URL-only provider composes one URL`() async throws { + let url = try #require(URL(string: "https://example.com/article")) + let extracted = await ShareContentExtractor.extract(from: [ + self.extensionItem(providers: [self.itemProvider(url as NSURL, type: .url)]), + ]) + + #expect(extracted.payload.url == url) + #expect(extracted.payload.text == nil) + #expect(ShareDraftComposer.compose(from: extracted.payload) == url.absoluteString) + #expect(extracted.attachmentSummary == ShareAttachmentSummary()) + } + + @Test @MainActor func `mirrored URL yields to later provider summary`() async throws { + let url = try #require(URL(string: "https://example.com/article")) + let extracted = await ShareContentExtractor.extract(from: [ + self.extensionItem(providers: [ + self.itemProvider(url as NSURL, type: .url), + self.itemProvider("Article summary" as NSString, type: .plainText), + ]), + ]) + + #expect(extracted.payload.url == url) + #expect(extracted.payload.text == "Article summary") + #expect(ShareDraftComposer.compose(from: extracted.payload) == + "Article summary\n\nhttps://example.com/article") + } + + @Test @MainActor func `provider extraction preserves attachment accounting`() async throws { + let url = try #require(URL(string: "https://example.com/article")) + let image = self.dataProvider(type: .image) + let extracted = await ShareContentExtractor.extract(from: [ + self.extensionItem(providers: [ + self.itemProvider(url as NSURL, type: .url), + self.itemProvider("Article summary" as NSString, type: .plainText), + image, + self.dataProvider(type: .movie), + self.dataProvider(type: .fileURL), + self.dataProvider(type: UTType(exportedAs: "ai.openclaw.tests.unknown")), + ]), + ]) + + #expect(extracted.imageProviders.count == 1) + #expect(extracted.imageProviders.first === image) + #expect(extracted.attachmentSummary == ShareAttachmentSummary( + selectedImageCount: 1, + acceptedImageCount: 0, + videoCount: 1, + fileCount: 1, + unknownCount: 1)) + } + + @MainActor + private func extensionItem(providers: [NSItemProvider]) -> NSExtensionItem { + let item = NSExtensionItem() + item.attachments = providers + return item + } + + private func dataProvider(type: UTType) -> NSItemProvider { + let provider = NSItemProvider() + provider.registerDataRepresentation(forTypeIdentifier: type.identifier, visibility: .all) { completion in + completion(Data([0x01]), nil) + return nil + } + return provider + } + + private func itemProvider(_ item: NSSecureCoding, type: UTType) -> NSItemProvider { + NSItemProvider(item: item, typeIdentifier: type.identifier) + } +} diff --git a/apps/ios/Tests/Logic/ShareDraftComposerTests.swift b/apps/ios/Tests/Logic/ShareDraftComposerTests.swift index 9daa01bca336..a9485e92bcf6 100644 --- a/apps/ios/Tests/Logic/ShareDraftComposerTests.swift +++ b/apps/ios/Tests/Logic/ShareDraftComposerTests.swift @@ -69,6 +69,37 @@ struct ShareDraftComposerTests { #expect(SharePayloadNormalizer.webURL(from: text) != nil) } + @Test func `URL mirror does not consume provider text admission`() throws { + let url = try #require(URL(string: "https://example.com/article")) + let mirrorOnly = SharePayloadNormalizer.distinctProviderText( + " https://example.com/article\n", + sharedURL: url) + #expect(mirrorOnly == nil) + #expect(ShareDraftComposer.compose(from: SharedContentPayload( + title: nil, + url: url, + text: mirrorOnly)) == "https://example.com/article") + + let text = mirrorOnly ?? SharePayloadNormalizer.distinctProviderText( + "Article summary", + sharedURL: url) + + #expect(text == "Article summary") + #expect(ShareDraftComposer.compose(from: SharedContentPayload( + title: nil, + url: url, + text: text)) == "Article summary\n\nhttps://example.com/article") + } + + @Test func `first distinct provider text remains authoritative`() throws { + let url = try #require(URL(string: "https://example.com/article")) + + let first = SharePayloadNormalizer.distinctProviderText("First provider text", sharedURL: url) + let retained = first ?? SharePayloadNormalizer.distinctProviderText("Later provider text", sharedURL: url) + + #expect(retained == "First provider text") + } + @Test func `deduplicates attributed content that mirrors provider text or URL`() throws { let url = try #require(URL(string: "https://example.com/article")) #expect(SharePayloadNormalizer.distinctAttributedText( diff --git a/apps/ios/project.yml b/apps/ios/project.yml index 66f6dead6d68..fc0002f37c4f 100644 --- a/apps/ios/project.yml +++ b/apps/ios/project.yml @@ -430,6 +430,8 @@ targets: - path: Tests/Logic - path: ShareExtension/ShareDraftComposer.swift group: ShareExtension + - path: ShareExtension/ShareContentExtractor.swift + group: ShareExtension - path: ShareExtension/ShareComposeState.swift group: ShareExtension - path: ShareExtension/ShareAttachmentSummary.swift