From 915b8e81233ad116e71c9944b6495522f662ff1b Mon Sep 17 00:00:00 2001 From: Stephan D Date: Fri, 23 Oct 2026 12:33:28 +0200 Subject: [PATCH] Restore web identity photo previews (#69) --- docs/ux/photo-preview.md | 79 ++++++++++ frontend/app/lib/kyc/preview.dart | 22 ++- frontend/app/lib/l10n/en.arb | 1 + frontend/app/lib/l10n/ru.arb | 1 + frontend/app/pubspec.lock | 2 +- frontend/app/pubspec.yaml | 1 + frontend/app/test/preview_test.dart | 229 ++++++++++++++++++++++++++++ 7 files changed, 330 insertions(+), 5 deletions(-) create mode 100644 docs/ux/photo-preview.md create mode 100644 frontend/app/test/preview_test.dart diff --git a/docs/ux/photo-preview.md b/docs/ux/photo-preview.md new file mode 100644 index 0000000..15be050 --- /dev/null +++ b/docs/ux/photo-preview.md @@ -0,0 +1,79 @@ +# Identity photo preview correction + +Task [#69](https://git.sendico.io/tech/sxchnge/issues/69), branch `kyc-69`. +This frontend change builds on the [identity verification journey](kyc.md) +and is independent of the birthday controls in task #67. + +## Cause and correction + +A synthetic valid PNG reproduced this Flutter 3.47.7 browser error: + +```text +TypeError: Instance of '_UnmodifiableUint8ListView': type +'_UnmodifiableUint8ListView' is not a subtype of type 'NativeUint8List' +``` + +The stack reached `Uint8ListToJSUint8Array.toJS` from CanvasKit's browser +image decoder through `MemoryImage` and `ResizeImage`. File selection had +successfully decoded the original typed bytes. The `Photo` constructor then +copied them into an unmodifiable view, which was passed directly into the +preview's web decoder. Native decoding accepted that view, hiding the bug in +native-only tests. The generic image error builder presented the failure as a +damaged document even though the selected bytes were valid. + +`PhotoPreview` now creates a native `Uint8List` copy for `MemoryImage` once per +selected photo. The shared domain model stays immutable, and validation, +providers, services and transport contracts remain unchanged. The preview still +requests a decode width of at most 1000 pixels and evicts its decoded cache entry +on replacement or disposal. The extra encoded-byte copy belongs to the preview +and is released with its provider/cache references; no secure-erasure guarantee +is made. + +An actual preview failure now says that the preview could not be displayed and +suggests selecting the photo again. The English/Russian fallback keeps at least +the preview's usual height, can grow for long text, and has a separate live-region +semantic node. Existing replacement/removal controls stay available and selected +input is retained. This is distinct from validation's corrupt-file message. + +## UX and measurement review + +The outcome remains selecting readable ID and selfie photos, checking both on +review, and submitting for confirmed service acceptance. Existing KYC journey +attempts, retries, failure, submission and abandonment signals are reused. No +control-level events, image bytes, filenames, form values or raw exceptions are +added to telemetry. A selection success measures validation; it does not establish +that pixels were painted. Preview-only rendering failures are not separately +counted by existing telemetry. Server review/approval measurement and native +screen-reader QA retain the limitations documented in the original journey. + +Regression tests use synthetic JPEG/PNG content only. They start and await real +image decoding outside the widget test's fake async zone, inspect the actual +rendered images, and surface stream errors so an error fallback cannot conceal a +failure. Checks cover portrait/landscape decoding, model immutability, bounded +decode width, stable provider reuse, replacement/removal cache eviction, both +review slots, returning to review, localized fallback/replacement, a 320px view +at 1.6 text scale, and error announcements isolated from surrounding instructions. +The `image` package is already a KYC runtime dependency; its direct app test +dependency is used only to create synthetic fixtures in memory. + +## Validation and delivery + +The original browser regression failed before the fix while its native equivalent +passed. The complete task diff was reviewed against frontend architecture, UX, +measurement, lifecycle and privacy requirements. Final validation passed: + +- In `frontend/app`, `dart format --output=none --set-exit-if-changed + lib/kyc/preview.dart test/preview_test.dart`: two files, no changes. +- In `frontend/app`, `flutter analyze --no-pub`: no issues. +- In `frontend/app`, `flutter test --platform chrome test/preview_test.dart + --reporter expanded --no-pub --verbose`: all six regression tests passed. +- At the repository root, `make test-frontend`: all 228 tests passed across + the shared packages, theme example and app, including the six native preview + regressions and existing KYC journey/analytics tests. +- In `frontend/app`, `flutter build web --no-pub`: release build and Wasm + compatibility dry run succeeded. + +Browser regression coverage uses Chrome; Safari and native assistive-technology +testing remain release checks. Generated localization and build code stays +ignored; the tracked app lockfile changes only `image` from transitive to direct +test dependency. The separate birthday worktree was preserved. diff --git a/frontend/app/lib/kyc/preview.dart b/frontend/app/lib/kyc/preview.dart index 89f527f..a1db853 100644 --- a/frontend/app/lib/kyc/preview.dart +++ b/frontend/app/lib/kyc/preview.dart @@ -1,3 +1,5 @@ +import 'dart:typed_data'; + import 'package:flutter/material.dart'; import 'package:sxchnge_kyc/sxchnge_kyc.dart'; @@ -15,8 +17,12 @@ class PhotoPreview extends StatefulWidget { class _PhotoPreviewState extends State { late ImageProvider _image = _provider(); - ImageProvider _provider() => - ResizeImage.resizeIfNeeded(1000, null, MemoryImage(widget.photo.bytes)); + ImageProvider _provider() => ResizeImage.resizeIfNeeded( + 1000, + null, + // Web decoders require a native typed array, not the model's read-only view. + MemoryImage(Uint8List.fromList(widget.photo.bytes)), + ); @override void didUpdateWidget(covariant PhotoPreview oldWidget) { super.didUpdateWidget(oldWidget); @@ -38,7 +44,15 @@ class _PhotoPreviewState extends State { height: 220, fit: BoxFit.contain, semanticLabel: widget.label, - errorBuilder: (_, _, _) => - Text(AppLocalizations.of(context)!.kycErrorUnreadable), + errorBuilder: (_, _, _) => ConstrainedBox( + constraints: const BoxConstraints(minHeight: 220), + child: Center( + child: Semantics( + container: true, + liveRegion: true, + child: Text(AppLocalizations.of(context)!.kycErrorPreview), + ), + ), + ), ); } diff --git a/frontend/app/lib/l10n/en.arb b/frontend/app/lib/l10n/en.arb index edfccf6..024c94b 100644 --- a/frontend/app/lib/l10n/en.arb +++ b/frontend/app/lib/l10n/en.arb @@ -128,6 +128,7 @@ "kycErrorFormat": "Choose a JPEG or PNG image. Other formats are not supported.", "kycErrorLarge": "This image is too large. Choose one under 10 MB and 40 megapixels.", "kycErrorUnreadable": "This image is damaged or cannot be read. Choose another photo.", + "kycErrorPreview": "The photo preview could not be displayed. Try choosing the photo again.", "kycErrorSmall": "The photo is too small. Use an image at least 600 pixels wide and high.", "kycErrorIncomplete": "Choose both photos and confirm they belong to you before submitting.", "kycErrorSession": "Your session has expired. Return to login, then check your verification status.", diff --git a/frontend/app/lib/l10n/ru.arb b/frontend/app/lib/l10n/ru.arb index 0ec126f..4b0069f 100644 --- a/frontend/app/lib/l10n/ru.arb +++ b/frontend/app/lib/l10n/ru.arb @@ -128,6 +128,7 @@ "kycErrorFormat": "Выберите изображение JPEG или PNG. Другие форматы не поддерживаются.", "kycErrorLarge": "Изображение слишком большое. Выберите файл до 10 МБ и 40 мегапикселей.", "kycErrorUnreadable": "Изображение повреждено или не читается. Выберите другое фото.", + "kycErrorPreview": "Не удалось показать фото. Попробуйте выбрать его ещё раз.", "kycErrorSmall": "Фото слишком маленькое. Ширина и высота должны быть не менее 600 пикселей.", "kycErrorIncomplete": "Выберите оба фото и подтвердите, что на них вы и ваш документ.", "kycErrorSession": "Сессия истекла. Войдите снова и проверьте статус верификации.", diff --git a/frontend/app/pubspec.lock b/frontend/app/pubspec.lock index 629743b..14fbf73 100644 --- a/frontend/app/pubspec.lock +++ b/frontend/app/pubspec.lock @@ -222,7 +222,7 @@ packages: source: hosted version: "4.1.2" image: - dependency: transitive + dependency: "direct dev" description: name: image sha256: "1976370a4df3091bb0f72409c187ad1f9132a818bc6b95ca59c0bae1c75c688e" diff --git a/frontend/app/pubspec.yaml b/frontend/app/pubspec.yaml index 592bd5e..15d1673 100644 --- a/frontend/app/pubspec.yaml +++ b/frontend/app/pubspec.yaml @@ -29,6 +29,7 @@ dependencies: go_router: ^18.0.1 country_picker: ^2.0.28 dev_dependencies: + image: ^4.9.0 flutter_test: sdk: flutter flutter_lints: ^6.0.0 diff --git a/frontend/app/test/preview_test.dart b/frontend/app/test/preview_test.dart new file mode 100644 index 0000000..e60a67b --- /dev/null +++ b/frontend/app/test/preview_test.dart @@ -0,0 +1,229 @@ +import 'dart:async'; +import 'dart:typed_data'; + +import 'package:flutter/material.dart'; + +import 'package:flutter_test/flutter_test.dart'; + +import 'package:image/image.dart' as images; + +import 'package:sxchnge_kyc/sxchnge_kyc.dart'; + +import 'package:sxchnge_app/kyc/photo.dart'; +import 'package:sxchnge_app/kyc/preview.dart'; + +import 'package:sxchnge_app/generated/i18n/app_localizations.dart'; + +Widget host(Widget child, {Locale locale = const Locale('en')}) => MaterialApp( + locale: locale, + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + home: Scaffold(body: SingleChildScrollView(child: child)), +); + +// Start decoding outside FakeAsync as well as waiting outside it, including +// browser codec callbacks that otherwise remain pending in the test zone. +Future show(WidgetTester tester, Widget widget) async { + await tester.runAsync(() => tester.pumpWidget(widget)); +} + +// pumpAndSettle alone does not wait for image codecs. Listen to the actual +// preview streams so a hidden errorBuilder cannot make this regression pass. +Future decoded(WidgetTester tester, {bool succeeds = true}) async { + final providers = tester + .widgetList(find.byType(Image)) + .map((image) => image.image) + .toList(); + expect(providers, isNotEmpty); + for (final provider in providers) { + Object? renderError; + StackTrace? renderStack; + await tester.runAsync(() async { + final stream = provider.resolve(ImageConfiguration.empty); + final ready = Completer(); + final listener = ImageStreamListener( + (info, _) { + info.dispose(); + if (!ready.isCompleted) ready.complete(); + }, + onError: (Object error, StackTrace? stack) { + renderError = error; + renderStack = stack; + if (!ready.isCompleted) ready.complete(); + }, + ); + stream.addListener(listener); + try { + await ready.future.timeout(const Duration(seconds: 15)); + } finally { + stream.removeListener(listener); + } + }); + expect( + renderError, + succeeds ? isNull : isNotNull, + reason: succeeds ? '$renderError\n$renderStack' : null, + ); + } + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); +} + +void main() { + late Uint8List png; + late Uint8List jpeg; + setUpAll(() { + // Synthetic, non-sensitive portrait and landscape photos in both formats. + png = images.encodePng(images.Image(width: 1200, height: 1600)); + jpeg = images.encodeJpg(images.Image(width: 1600, height: 1200)); + }); + + for (final format in ['PNG', 'JPEG']) { + testWidgets('accepted $format renders from an immutable photo model', ( + tester, + ) async { + final source = Uint8List.fromList(format == 'PNG' ? png : jpeg); + final photo = (await tester.runAsync( + () => FilePhotoService.validate(source), + ))!; + final originalFirstByte = photo.bytes.first; + source[0] = 0; + expect(photo.bytes.first, originalFirstByte); + expect(() => photo.bytes[0] = 0, throwsUnsupportedError); + await show( + tester, + host(PhotoPreview(photo: photo, label: 'ID document photo')), + ); + await decoded(tester); + final raw = tester.widget(find.byType(RawImage)); + expect(raw.image, isNotNull); + expect(raw.image!.width, lessThanOrEqualTo(1000)); + expect( + tester.widget(find.byType(Image)).semanticLabel, + 'ID document photo', + ); + expect(find.textContaining('damaged'), findsNothing); + expect(find.textContaining('could not be displayed'), findsNothing); + }); + } + + testWidgets('replacement and removal evict sensitive preview cache entries', ( + tester, + ) async { + final first = Photo(png); + await show(tester, host(PhotoPreview(photo: first, label: 'ID'))); + await decoded(tester); + final initialProvider = tester.widget(find.byType(Image)).image; + final initialKey = await initialProvider.obtainKey( + ImageConfiguration.empty, + ); + final cache = PaintingBinding.instance.imageCache; + expect(cache.statusForKey(initialKey).keepAlive, isTrue); + await show(tester, host(PhotoPreview(photo: first, label: 'Document'))); + await decoded(tester); + expect( + identical( + tester.widget(find.byType(Image)).image, + initialProvider, + ), + isTrue, + ); + await show(tester, host(PhotoPreview(photo: Photo(jpeg), label: 'ID'))); + await decoded(tester); + expect(cache.statusForKey(initialKey).untracked, isTrue); + final replacement = tester.widget(find.byType(Image)).image; + final replacementKey = await replacement.obtainKey( + ImageConfiguration.empty, + ); + expect(cache.statusForKey(replacementKey).keepAlive, isTrue); + expect(tester.widget(find.byType(RawImage)).image!.width, 1000); + await show(tester, host(const SizedBox())); + await tester.pumpAndSettle(); + expect(cache.statusForKey(replacementKey).untracked, isTrue); + }); + + testWidgets('ID and selfie both render on review and after returning', ( + tester, + ) async { + final passport = Photo(png); + final selfie = Photo(jpeg); + for (var visit = 0; visit < 2; visit++) { + await show( + tester, + host( + Column( + children: [ + PhotoPanel( + kind: PhotoKind.passport, + photo: passport, + review: true, + ), + PhotoPanel(kind: PhotoKind.selfie, photo: selfie, review: true), + ], + ), + ), + ); + await decoded(tester); + final previews = tester.widgetList(find.byType(RawImage)); + expect(previews.length, 2); + expect(previews.every((image) => image.image != null), isTrue); + expect(find.text('Choose a different photo'), findsNothing); + await show(tester, host(const SizedBox())); + await tester.pumpAndSettle(); + } + }); + + for (final locale in ['en', 'ru']) { + testWidgets( + '$locale preview failure explains recovery and can be replaced', + (tester) async { + tester.view.physicalSize = const Size(320, 900); + tester.view.devicePixelRatio = 1; + tester.platformDispatcher.textScaleFactorTestValue = 1.6; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); + var replacements = 0; + var removals = 0; + await show( + tester, + host( + PhotoPanel( + kind: PhotoKind.passport, + photo: Photo(Uint8List.fromList([1, 2, 3])), + onPick: () => replacements++, + onRemove: () => removals++, + ), + locale: Locale(locale), + ), + ); + await decoded(tester, succeeds: false); + final l = AppLocalizations.of(tester.element(find.byType(PhotoPanel)))!; + expect(find.text(l.kycErrorPreview), findsOneWidget); + expect(find.text(l.kycErrorUnreadable), findsNothing); + expect( + tester.getSemantics(find.text(l.kycErrorPreview)), + matchesSemantics(isLiveRegion: true, label: l.kycErrorPreview), + ); + final replace = find.byKey(const ValueKey('pick-passport')); + await tester.ensureVisible(replace); + await tester.tap(replace); + final remove = find.text(l.kycRemove); + await tester.ensureVisible(remove); + await tester.tap(remove); + expect(replacements, 1); + expect(removals, 1); + await show( + tester, + host( + PhotoPanel(kind: PhotoKind.passport, photo: Photo(jpeg)), + locale: Locale(locale), + ), + ); + await decoded(tester); + expect(find.text(l.kycErrorPreview), findsNothing); + expect(tester.widget(find.byType(RawImage)).image, isNotNull); + }, + ); + } +}