diff --git a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzNotificationAvatarLoader.swift b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzNotificationAvatarLoader.swift index 81335dcdae..cc46264a84 100644 --- a/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzNotificationAvatarLoader.swift +++ b/mobile/ios/BuzzPushKit/Sources/BuzzPushKit/BuzzNotificationAvatarLoader.swift @@ -54,11 +54,19 @@ public final class BuzzNotificationAvatarLoader { completion(.failure(BuzzNotificationCacheError.invalidRecord)) return } - let thumbnail = Self.thumbnailURL(original, scope: scope) - fetch(thumbnail ?? original, scope: scope) { result in - if thumbnail != nil, case .failure(BuzzNotificationDownload.Failure.status(404)) = result { - // Older or externally populated media may not have a generated thumbnail. - self.fetch(original, scope: scope, completion: completion) + fetchFirst( + Self.thumbnailURLs(original, scope: scope) + [original], scope: scope, completion: completion) + } + + /// Fetches the first of `urls` the relay has: older relays, and media uploaded before WebP + /// thumbnails, have only the JPEG one, and externally populated media may have neither. + private func fetchFirst( + _ urls: [URL], scope: BuzzNotificationCacheScope, + completion: @escaping (Result) -> Void + ) { + fetch(urls[0], scope: scope) { result in + if urls.count > 1, case .failure(BuzzNotificationDownload.Failure.status(404)) = result { + self.fetchFirst(Array(urls.dropFirst()), scope: scope, completion: completion) } else { completion(result) } @@ -173,14 +181,17 @@ public final class BuzzNotificationAvatarLoader { return components.url } - static func thumbnailURL(_ url: URL, scope: BuzzNotificationCacheScope) -> URL? { - guard isRelayMedia(url, scope: scope) else { return nil } + /// The relay's thumbnails, best first: the WebP keeps transparency; every relay writes the JPEG. + static func thumbnailURLs(_ url: URL, scope: BuzzNotificationCacheScope) -> [URL] { + guard isRelayMedia(url, scope: scope) else { return [] } let parts = url.lastPathComponent.split(separator: ".") guard let hash = parts.first, hash.count == 64, VerifiedNostrEvent.hexBytes(String(hash))?.count == 32, parts.count <= 2 - else { return nil } - return url.deletingLastPathComponent().appendingPathComponent("\(hash).thumb.jpg") + else { return [] } + return ["webp", "jpg"].map { + url.deletingLastPathComponent().appendingPathComponent("\(hash).thumb.\($0)") + } } private static func isRelayMedia(_ url: URL, scope: BuzzNotificationCacheScope) -> Bool { diff --git a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzNotificationAvatarLoaderTests.swift b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzNotificationAvatarLoaderTests.swift index 0e79c2c585..8a00e7a62f 100644 --- a/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzNotificationAvatarLoaderTests.swift +++ b/mobile/ios/BuzzPushKit/Tests/BuzzPushKitTests/BuzzNotificationAvatarLoaderTests.swift @@ -39,7 +39,9 @@ struct BuzzNotificationAvatarLoaderTests { id: "community", name: "Test", relayUrl: relay, pubkey: "account", policies: [], notificationSessionID: "session"))) let url = try #require(URL(string: "\(media)/media/\(hash).png")) - #expect((BuzzNotificationAvatarLoader.thumbnailURL(url, scope: scope) != nil) == expected) + #expect( + BuzzNotificationAvatarLoader.thumbnailURLs(url, scope: scope).map(\.lastPathComponent) + == (expected ? ["\(hash).thumb.webp", "\(hash).thumb.jpg"] : [])) } } diff --git a/mobile/lib/shared/relay/media_image.dart b/mobile/lib/shared/relay/media_image.dart index 43ba776486..7608cef60b 100644 --- a/mobile/lib/shared/relay/media_image.dart +++ b/mobile/lib/shared/relay/media_image.dart @@ -368,19 +368,23 @@ class MediaImageCooldownException implements Exception { final _relayBlobPath = RegExp(r'^/media/([0-9a-f]{64})(?:\.[a-z0-9]{1,8})?$'); -/// The relay's generated thumbnail for a relay-hosted image, or null when -/// [url] is not one. Every image uploaded to a relay gets a JPEG thumbnail at -/// `/media/.thumb.jpg`, at most 320px on its longer side. This -/// matches desktop's small avatars (buzz-app `mediaUrl`), including the cost: -/// the thumbnail is a still JPEG, so transparent areas render black and -/// animation is lost. -String? relayThumbnailUrl(String url, MediaGetAuthService auth) { - if (!auth.isRelayMediaUrl(url)) return null; +/// The relay's generated thumbnails for a relay-hosted image, best first, or +/// empty when [url] is not one. Relays write `/media/.thumb.webp`, +/// which keeps transparency, beside the `.thumb.jpg` that every relay +/// writes. Older relays, and images uploaded before WebP thumbnails, have +/// only the JPEG. Both are at most 320px on their longer side and still, so +/// animation is lost. This matches desktop's small avatars (buzz-app +/// `mediaUrl`). +List relayThumbnailUrls(String url, MediaGetAuthService auth) { + if (!auth.isRelayMediaUrl(url)) return const []; final uri = Uri.parse(url); - if (uri.hasQuery || uri.hasFragment) return null; + if (uri.hasQuery || uri.hasFragment) return const []; final match = _relayBlobPath.firstMatch(uri.path); - if (match == null) return null; - return uri.replace(path: '/media/${match[1]}.thumb.jpg').toString(); + if (match == null) return const []; + return [ + for (final ext in ['webp', 'jpg']) + uri.replace(path: '/media/${match[1]}.thumb.$ext').toString(), + ]; } /// Drop-in replacement for the media `Image.network` call sites. @@ -397,7 +401,7 @@ String? relayThumbnailUrl(String url, MediaGetAuthService auth) { /// /// With [preferThumbnail], a relay image laid out small enough loads the /// relay's generated thumbnail instead of the original upload, falling back -/// to the original when the relay has no thumbnail for it. +/// to the JPEG thumbnail and then the original when the relay has none. class MediaImage extends ConsumerWidget { final String url; final BoxFit? fit; @@ -475,27 +479,31 @@ class MediaImage extends ConsumerWidget { final cacheWidth = maxWidth.isFinite && maxWidth > 0 ? _physicalWidth(context, maxWidth) : null; - final candidate = + final thumbnails = preferThumbnail && _fitsThumbnail(context, constraints) - ? relayThumbnailUrl(url, auth) - : null; - final thumbnail = _missingThumbnails.contains(candidate) - ? null - : candidate; - if (thumbnail == null) return _image(providerFor(url), cacheWidth); - return _image( - providerFor(thumbnail, isThumbnailFallback: true), - cacheWidth, - errorBuilder: (context, error, stackTrace) { - if (error is! NetworkImageLoadException || - error.statusCode != 404) { - return errorBuilder!(context, error, stackTrace); - } - // The fetch already recorded the missing thumbnail, so later - // builds go straight to the original. - return _image(providerFor(url), cacheWidth); - }, - ); + ? relayThumbnailUrls(url, auth) + : const []; + Widget source(Iterable candidates) { + final rest = candidates + .skipWhile(_missingThumbnails.contains) + .toList(); + if (rest.isEmpty) return _image(providerFor(url), cacheWidth); + return _image( + providerFor(rest.first, isThumbnailFallback: true), + cacheWidth, + errorBuilder: (context, error, stackTrace) { + if (error is! NetworkImageLoadException || + error.statusCode != 404) { + return errorBuilder!(context, error, stackTrace); + } + // The fetch already recorded the missing thumbnail, so later + // builds go straight to the next source. + return source(rest.skip(1)); + }, + ); + } + + return source(thumbnails); }, ); } diff --git a/mobile/test/shared/relay/media_image_test.dart b/mobile/test/shared/relay/media_image_test.dart index 91dbb7d912..789fa9dd02 100644 --- a/mobile/test/shared/relay/media_image_test.dart +++ b/mobile/test/shared/relay/media_image_test.dart @@ -73,29 +73,33 @@ void main() { }); }); - group('relayThumbnailUrl', () { + group('relayThumbnailUrls', () { final hash = 'ab' * 32; final auth = _auth(); - test('maps relay blobs of any format to the thumbnail', () { - for (final ext in ['.jpg', '.jpeg', '.png', '.webp', '.gif', '']) { - expect( - relayThumbnailUrl('$_relayBase/media/$hash$ext', auth), - '$_relayBase/media/$hash.thumb.jpg', - ); - } - }); + test( + 'maps relay blobs of any format to the WebP, then JPEG, thumbnail', + () { + for (final ext in ['.jpg', '.jpeg', '.png', '.webp', '.gif', '']) { + expect(relayThumbnailUrls('$_relayBase/media/$hash$ext', auth), [ + '$_relayBase/media/$hash.thumb.webp', + '$_relayBase/media/$hash.thumb.jpg', + ]); + } + }, + ); test('leaves everything else alone', () { for (final url in [ 'https://elsewhere.example/media/$hash.jpg', '$_relayBase/media/$hash.thumb.jpg', + '$_relayBase/media/$hash.thumb.webp', '$_relayBase/media/$hash.jpg?v=2', '$_relayBase/media/not-a-hash.jpg', '$_relayBase/avatars/$hash.jpg', '$_relayBase/media/$hash.JPG', ]) { - expect(relayThumbnailUrl(url, auth), isNull, reason: url); + expect(relayThumbnailUrls(url, auth), isEmpty, reason: url); } }); }); diff --git a/mobile/test/shared/widgets/avatar_image_test.dart b/mobile/test/shared/widgets/avatar_image_test.dart index c0d15f105d..2c885c532e 100644 --- a/mobile/test/shared/widgets/avatar_image_test.dart +++ b/mobile/test/shared/widgets/avatar_image_test.dart @@ -109,10 +109,12 @@ void main() { final provider = (image.image as ResizeImage).imageProvider; return (provider as MediaImageProvider).url; }); - expect(shown, ['$relay/media/$hash.thumb.jpg', original]); + expect(shown, ['$relay/media/$hash.thumb.webp', original]); }); - testWidgets('a missing thumbnail falls back to the original', (tester) async { + testWidgets('missing thumbnails fall back to the JPEG, then the original', ( + tester, + ) async { const relay = 'https://relay.example'; final missing = '$relay/media/${'cd' * 32}.jpg'; final broken = '$relay/media/${'ef' * 32}.jpg'; @@ -157,9 +159,19 @@ void main() { await tester.pump(); } - String thumb(String url) => url.replaceFirst('.jpg', '.thumb.jpg'); - // A 404 retries the original; any other failure shows the fallback. - expect(requested, containsAll([thumb(missing), missing, thumb(broken)])); + String thumb(String url, String ext) => + url.replaceFirst('.jpg', '.thumb.$ext'); + // A 404 tries the next source; any other failure shows the fallback. + expect( + requested, + containsAll([ + thumb(missing, 'webp'), + thumb(missing, 'jpg'), + missing, + thumb(broken, 'webp'), + ]), + ); + expect(requested, isNot(contains(thumb(broken, 'jpg')))); expect(requested, isNot(contains(broken))); expect(find.text('B'), findsOneWidget); }); @@ -213,7 +225,7 @@ void main() { expect(MediaImage.debugMissingThumbnailCount, 256); expect( MediaImageProvider.debugCooldownUrls.where( - (url) => url.endsWith('.thumb.jpg'), + (url) => url.contains('.thumb.'), ), isEmpty, ); @@ -243,9 +255,11 @@ void main() { }) async { tester.view.devicePixelRatio = 3; addTearDown(tester.view.resetDevicePixelRatio); - final client = http_testing.MockClient((request) { + final client = http_testing.MockClient((request) async { final url = request.url.toString(); requested.add(url); + // These journeys run against a relay from before WebP thumbnails. + if (url.endsWith('.thumb.webp')) return http.Response('', 404); return respond(url); }); await tester.pumpWidget( @@ -306,7 +320,7 @@ void main() { fallback: const Text('M'), ), ); - expect(requested, [thumbnail]); + expect(requested, [thumbnail.replaceFirst('.jpg', '.webp'), thumbnail]); await show(tester, const SizedBox()); slow404.complete(http.Response('', 404)); await show(tester, const SizedBox());