From 220cec677a324884ea0bd10ee52cdcf29867e17d Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Sun, 1 Nov 2026 15:51:13 -0800 Subject: [PATCH] fix(mobile): keep community icons on originals and record thumbnail 404s at fetch (#8896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #8879, addressing its review. ## Changes - **Community icons load the original again.** `AvatarImageContent` gained a `preferThumbnail` flag, off by default. People and agent avatars turn it on (as #8879 did for every caller); `CommunityAvatar` leaves it off. Community logos are often transparent PNGs, and the relay's JPEG thumbnail renders transparency black. This matches desktop, which shows community icons from the original. - **A missing thumbnail is recorded where its 404 arrives.** Before, only the mounted `Image`'s error builder recorded it. If the avatar scrolled away before a slow 404 came back, the thumbnail URL kept a 30s failure cooldown instead, and showing that avatar again at any size during the cooldown rendered initials and never requested the original. The fetch now records a 404 on a relay `/media/.thumb.jpg` URL in the missing-thumbnail set and puts no cooldown on it. ## Tests Two journey tests in `avatar_image_test.dart`, both failing on main and passing here: - An avatar unmounts before its slow thumbnail 404 arrives, then mounts again at another size: it requests the original once, renders it (valid image bytes, no initials), and leaves no cooldown. - A 36pt `CommunityAvatar` with a relay-hosted icon requests and shows only the original. `flutter analyze` is clean. In the full local `flutter test` run, only 12 push and notification tests fail, and they fail the same way on main in this environment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow Co-authored-by: Tom Brow Co-authored-by: Claude Opus 5.5 --- .../features/agents/agent_avatar_picker.dart | 3 + mobile/lib/features/agents/agent_card.dart | 2 + .../profile/settings_profile_header.dart | 1 + .../features/profile/user_profile_sheet.dart | 3 + mobile/lib/shared/relay/media_image.dart | 37 ++-- .../shared/widgets/agent_artwork_panel.dart | 1 + mobile/lib/shared/widgets/avatar_image.dart | 10 +- .../shared/widgets/preset_agent_avatar.dart | 7 +- .../widgets/progressive_animated_avatar.dart | 1 + .../shared/widgets/avatar_image_test.dart | 165 ++++++++++++++++++ 10 files changed, 218 insertions(+), 12 deletions(-) diff --git a/mobile/lib/features/agents/agent_avatar_picker.dart b/mobile/lib/features/agents/agent_avatar_picker.dart index c9d98808d8..b8500a801f 100644 --- a/mobile/lib/features/agents/agent_avatar_picker.dart +++ b/mobile/lib/features/agents/agent_avatar_picker.dart @@ -3,7 +3,9 @@ import 'dart:math' as math; import 'package:flutter/material.dart'; import 'package:flutter/foundation.dart'; + import '../../shared/widgets/ios_native_segmented_control.dart'; + import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; @@ -311,6 +313,7 @@ class AgentAvatarPicker extends HookConsumerWidget { ), child: AvatarImageContent( imageUrl: agentAvatarCatalog[i].url, + preferThumbnail: true, fallback: const Icon(Icons.smart_toy_outlined), ), ), diff --git a/mobile/lib/features/agents/agent_card.dart b/mobile/lib/features/agents/agent_card.dart index 24cb4debc5..dd77900804 100644 --- a/mobile/lib/features/agents/agent_card.dart +++ b/mobile/lib/features/agents/agent_card.dart @@ -59,6 +59,7 @@ class AgentCard extends StatelessWidget { ) : AvatarImageContent( imageUrl: poster, + preferThumbnail: true, fallback: fallback, fit: portraitFit, ); @@ -89,6 +90,7 @@ class AgentCard extends StatelessWidget { scale: 1.8, child: AvatarImageContent( imageUrl: poster, + preferThumbnail: true, fallback: const SizedBox.shrink(), ), ), diff --git a/mobile/lib/features/profile/settings_profile_header.dart b/mobile/lib/features/profile/settings_profile_header.dart index 4182d1dcf1..41f3022303 100644 --- a/mobile/lib/features/profile/settings_profile_header.dart +++ b/mobile/lib/features/profile/settings_profile_header.dart @@ -96,6 +96,7 @@ class SettingsProfileHeader extends HookConsumerWidget { : activeHandoff == null || animatedPosterUrl == null ? AvatarImageContent( imageUrl: avatarUrl, + preferThumbnail: true, fallback: _AvatarFallback(initial: profile?.initial), ) : Stack( diff --git a/mobile/lib/features/profile/user_profile_sheet.dart b/mobile/lib/features/profile/user_profile_sheet.dart index db79f98375..3472eb2e8e 100644 --- a/mobile/lib/features/profile/user_profile_sheet.dart +++ b/mobile/lib/features/profile/user_profile_sheet.dart @@ -1,8 +1,10 @@ import 'dart:async'; import 'package:flutter/material.dart'; + import '../../shared/widgets/profile_action_chip.dart'; import '../../shared/widgets/app_list_card.dart'; + import 'package:flutter/services.dart'; import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; @@ -602,6 +604,7 @@ class _ProfileAvatar extends HookWidget { ) : AvatarImageContent( imageUrl: animatedAvatar?.posterUrl ?? avatarUrl, + preferThumbnail: true, fallback: _AvatarFallback(initial: initial), ); return AvatarClip( diff --git a/mobile/lib/shared/relay/media_image.dart b/mobile/lib/shared/relay/media_image.dart index 8242d4f0b1..7d8f016fdb 100644 --- a/mobile/lib/shared/relay/media_image.dart +++ b/mobile/lib/shared/relay/media_image.dart @@ -45,6 +45,13 @@ class MediaImageProvider extends ImageProvider { /// Excluded from equality because it is a side effect, not cache identity. final ValueChanged? onBytesLoaded; + /// Set when [url] is a thumbnail [MediaImage] chose in place of an + /// original: its 404 then records a missing thumbnail, so later builds load + /// the original, rather than a failure cooldown. Any other 404, including a + /// profile URL that is itself a thumbnail, keeps the cooldown. Excluded from + /// equality: it changes how a failure is handled, not the bytes. + final bool isThumbnailFallback; + /// Excluded from equality: transport, not identity. final http.Client client; @@ -62,6 +69,7 @@ class MediaImageProvider extends ImageProvider { required this.auth, required this.client, this.onBytesLoaded, + this.isThumbnailFallback = false, this.scale = 1.0, this.stallTimeout = const Duration(seconds: 20), this.maxFetchDuration = const Duration(minutes: 5), @@ -240,6 +248,14 @@ class MediaImageProvider extends ImageProvider { } rethrow; } + if (response.statusCode == 404 && isThumbnailFallback) { + // Older or externally populated media may have no generated thumbnail. + // Record that here rather than in a widget's error builder: the widget + // that asked may be gone by the time a slow 404 arrives, and a + // cooldown would then keep every later build off the original. + MediaImage._rememberMissingThumbnail(url); + throw NetworkImageLoadException(statusCode: 404, uri: uri); + } if (response.statusCode != 200) { _cooldownUntil[url] = debugNow().add(_cooldownFor(response)); throw NetworkImageLoadException( @@ -468,11 +484,15 @@ class MediaImage extends ConsumerWidget { Widget build(BuildContext context, WidgetRef ref) { final auth = ref.watch(mediaGetAuthServiceProvider); final client = ref.watch(mediaHttpClientProvider); - MediaImageProvider providerFor(String source) => MediaImageProvider( + MediaImageProvider providerFor( + String source, { + bool isThumbnailFallback = false, + }) => MediaImageProvider( url: source, auth: auth, client: client, onBytesLoaded: onBytesLoaded, + isThumbnailFallback: isThumbnailFallback, ); if (decodeWidth != null) { @@ -496,17 +516,15 @@ class MediaImage extends ConsumerWidget { : candidate; if (thumbnail == null) return _image(providerFor(url), cacheWidth); return _image( - providerFor(thumbnail), + providerFor(thumbnail, isThumbnailFallback: true), cacheWidth, errorBuilder: (context, error, stackTrace) { if (error is! NetworkImageLoadException || error.statusCode != 404) { return errorBuilder!(context, error, stackTrace); } - // Older or externally populated media may have no generated - // thumbnail. Remember that, so later builds go straight to the - // original instead of hitting the thumbnail's failure cooldown. - _rememberMissingThumbnail(thumbnail); + // The fetch already recorded the missing thumbnail, so later + // builds go straight to the original. return _image(providerFor(url), cacheWidth); }, ); @@ -520,11 +538,10 @@ class MediaImage extends ConsumerWidget { static final Set _missingThumbnails = {}; static const _maxMissingThumbnails = 256; + /// Called by the fetch that got the 404, which puts no failure cooldown on + /// the thumbnail: this set stands in for it, and an entry evicted here then + /// costs one more 404 rather than showing the fallback. static void _rememberMissingThumbnail(String url) { - // This set now stands in for the 404's failure cooldown. Drop that, or - // nothing would fetch the URL again to clear it, and an entry evicted - // here during its cooldown would show the fallback, not the original. - MediaImageProvider._cooldownUntil.remove(url); _missingThumbnails ..remove(url) ..add(url); diff --git a/mobile/lib/shared/widgets/agent_artwork_panel.dart b/mobile/lib/shared/widgets/agent_artwork_panel.dart index 8906ab2e49..20e0217906 100644 --- a/mobile/lib/shared/widgets/agent_artwork_panel.dart +++ b/mobile/lib/shared/widgets/agent_artwork_panel.dart @@ -49,6 +49,7 @@ class AgentArtworkPanel extends StatelessWidget { scale: 2, child: AvatarImageContent( imageUrl: poster, + preferThumbnail: true, fallback: const SizedBox.shrink(), ), ), diff --git a/mobile/lib/shared/widgets/avatar_image.dart b/mobile/lib/shared/widgets/avatar_image.dart index cd05026af2..4f3e139cb4 100644 --- a/mobile/lib/shared/widgets/avatar_image.dart +++ b/mobile/lib/shared/widgets/avatar_image.dart @@ -45,6 +45,7 @@ class AvatarImage extends StatelessWidget { child: AvatarImageContent( imageUrl: animatedAvatar?.posterUrl ?? imageUrl, fallback: fallback, + preferThumbnail: true, ), ); final clipped = AvatarClip( @@ -77,11 +78,18 @@ class AvatarImageContent extends ConsumerStatefulWidget { final Widget fallback; final BoxFit fit; + /// Load the relay's small JPEG thumbnail for a small relay-hosted image + /// (see [MediaImage.preferThumbnail]). People and agent avatars set this, + /// matching desktop. Leave it off for images that may be transparent, such + /// as community icons: the thumbnail renders transparent areas black. + final bool preferThumbnail; + const AvatarImageContent({ super.key, required this.imageUrl, required this.fallback, this.fit = BoxFit.cover, + this.preferThumbnail = false, }); @override @@ -132,7 +140,7 @@ class _AvatarImageContentState extends ConsumerState { _NetworkAvatarSource(:final url) => MediaImage( url: url, fit: widget.fit, - preferThumbnail: true, + preferThumbnail: widget.preferThumbnail, errorBuilder: (_, _, _) => centeredFallback, ), null => centeredFallback, diff --git a/mobile/lib/shared/widgets/preset_agent_avatar.dart b/mobile/lib/shared/widgets/preset_agent_avatar.dart index 1b6dea74b2..6231948516 100644 --- a/mobile/lib/shared/widgets/preset_agent_avatar.dart +++ b/mobile/lib/shared/widgets/preset_agent_avatar.dart @@ -110,12 +110,17 @@ class PresetAgentAvatar extends HookWidget { playing: playing, fallback: AvatarImageContent( imageUrl: poster, + preferThumbnail: true, fallback: fallback, ), ) : controller != null && ready.value == controller ? VideoPlayer(controller) - : AvatarImageContent(imageUrl: poster, fallback: fallback), + : AvatarImageContent( + imageUrl: poster, + preferThumbnail: true, + fallback: fallback, + ), ), ), ); diff --git a/mobile/lib/shared/widgets/progressive_animated_avatar.dart b/mobile/lib/shared/widgets/progressive_animated_avatar.dart index ed0c54e089..20873d26a7 100644 --- a/mobile/lib/shared/widgets/progressive_animated_avatar.dart +++ b/mobile/lib/shared/widgets/progressive_animated_avatar.dart @@ -80,6 +80,7 @@ class ProgressiveAnimatedAvatar extends HookWidget { AvatarImageContent( key: const ValueKey('progressive-animated-avatar-poster'), imageUrl: descriptor.posterUrl, + preferThumbnail: true, fallback: fallback, fit: fit, ), diff --git a/mobile/test/shared/widgets/avatar_image_test.dart b/mobile/test/shared/widgets/avatar_image_test.dart index 0583e488b7..4a6f68a6ea 100644 --- a/mobile/test/shared/widgets/avatar_image_test.dart +++ b/mobile/test/shared/widgets/avatar_image_test.dart @@ -1,5 +1,8 @@ +import 'dart:async'; import 'dart:convert'; +import 'package:buzz/shared/community/community_avatar.dart'; +import 'package:buzz/shared/community/community_icon_provider.dart'; import 'package:buzz/shared/relay/media_auth.dart'; import 'package:buzz/shared/relay/media_image.dart'; import 'package:buzz/shared/widgets/avatar_shape.dart'; @@ -9,9 +12,16 @@ import 'package:flutter/material.dart'; import 'package:flutter_svg/flutter_svg.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:hooks_riverpod/misc.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart' as http_testing; +// Minimal valid 1x1 transparent PNG. +final _pngBytes = base64Decode( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAA' + 'AAYAAjCB0C8AAAAASUVORK5CYII=', +); + void main() { const svg = '' @@ -211,6 +221,161 @@ void main() { ); }); + group('relay thumbnail journeys', () { + const relay = 'https://relay.example'; + late List requested; + late Future Function(String url) respond; + final valueOf = ValueNotifier(const SizedBox()); + + setUp(() { + MediaImageProvider.debugResetCooldowns(); + MediaImage.debugResetMissingThumbnails(); + requested = []; + respond = (_) async => http.Response.bytes(_pngBytes, 200); + valueOf.value = const SizedBox(); + }); + tearDown(() { + MediaImageProvider.debugResetCooldowns(); + MediaImage.debugResetMissingThumbnails(); + }); + + Future mount( + WidgetTester tester, { + List overrides = const [], + }) async { + tester.view.devicePixelRatio = 3; + addTearDown(tester.view.resetDevicePixelRatio); + final client = http_testing.MockClient((request) { + final url = request.url.toString(); + requested.add(url); + return respond(url); + }); + await tester.pumpWidget( + ProviderScope( + overrides: [ + mediaGetAuthServiceProvider.overrideWithValue( + MediaGetAuthService(baseUrl: relay, nsec: null), + ), + mediaHttpClientProvider.overrideWithValue(client), + ...overrides, + ], + child: MaterialApp( + home: Center( + child: ValueListenableBuilder( + valueListenable: valueOf, + builder: (_, child, _) => child, + ), + ), + ), + ), + ); + } + + Future show(WidgetTester tester, Widget child) async { + valueOf.value = child; + for (var i = 0; i < 4; i++) { + await tester.runAsync( + () => Future.delayed(const Duration(milliseconds: 20)), + ); + await tester.pump(); + } + } + + String shownUrl(WidgetTester tester) { + final image = tester.widget(find.byType(Image)); + final provider = (image.image as ResizeImage).imageProvider; + return (provider as MediaImageProvider).url; + } + + testWidgets( + 'an avatar that left before its thumbnail 404 later shows the original', + (tester) async { + final original = '$relay/media/${'12' * 32}.png'; + final thumbnail = '$relay/media/${'12' * 32}.thumb.jpg'; + final slow404 = Completer(); + respond = (url) async => url == thumbnail + ? slow404.future + : http.Response.bytes(_pngBytes, 200); + await mount(tester); + + // A 40pt avatar asks for the thumbnail on a slow link, then + // scrolls away before the relay's 404 arrives. + await show( + tester, + AvatarImage( + imageUrl: original, + radius: 20, + fallback: const Text('M'), + ), + ); + expect(requested, [thumbnail]); + await show(tester, const SizedBox()); + slow404.complete(http.Response('', 404)); + await show(tester, const SizedBox()); + + // Back at another size, it loads and renders the original rather + // than the initials. + await show( + tester, + AvatarImage( + imageUrl: original, + radius: 16, + fallback: const Text('M'), + ), + ); + expect(requested.where((url) => url == original), hasLength(1)); + expect(shownUrl(tester), original); + expect(find.text('M'), findsNothing); + expect(MediaImageProvider.debugCooldownUrls, isEmpty); + + // A profile URL that is itself thumbnail-shaped, on another host or + // this relay, is not a fallback MediaImage chose: its 404 keeps the + // cooldown, so showing it again does not request it again. + respond = (_) async => http.Response('', 404); + for (final (i, thumbShaped) in [ + 'https://elsewhere.example/media/${'56' * 32}.thumb.jpg', + '$relay/media/${'78' * 32}.thumb.jpg', + ].indexed) { + MediaImageProvider.debugResetCooldowns(); + for (final radius in [16.0, 17.0]) { + await show( + tester, + AvatarImage( + key: ValueKey((i, radius)), + imageUrl: thumbShaped, + radius: radius, + fallback: const Text('E'), + ), + ); + expect(find.text('E'), findsOneWidget); + } + expect(requested.where((url) => url == thumbShaped), hasLength(1)); + expect(MediaImageProvider.debugCooldownUrls, [thumbShaped]); + } + }, + ); + + testWidgets('community icons load the original, which keeps transparency', ( + tester, + ) async { + final icon = '$relay/media/${'34' * 32}.png'; + await mount( + tester, + overrides: [ + communityIconPresentationProvider(relay).overrideWithValue(icon), + ], + ); + + // The channels header size, well within the thumbnail. + await show( + tester, + const CommunityAvatar(name: 'Buzz', relayUrl: relay, size: 36), + ); + expect(requested, [icon]); + expect(shownUrl(tester), icon); + }); + }); + testWidgets('clips agents to the desktop squircle', (tester) async { await tester.pumpWidget(subject(null, isAgent: true));