Record final photo preview contribution review (#69)

docs/ux/photo-preview.md

@@ -0,0 +1,98 @@
+# Identity photo preview correction
+
+Task [#69](https://git.sendico.io/tech/sxchnge/issues/69), branch `kyc-69`.
+Delivery: [PR #71](https://git.sendico.io/tech/sxchnge/pulls/71).
+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.
+
+## Follow-up contribution review
+
+The complete seven-file contribution was reviewed again before the authorized
+merge and frontend rollout. No additional defects were found in the byte-copy
+boundary, provider reuse, cache disposal, error semantics/localization, privacy,
+architecture or regression coverage. No source changes were needed.
+
+Current `main` (`ec4115d`, including the birthday, signup-delivery and account
+setup changes) was merged into `kyc-69` before final verification. The two-file
+format check and app analyzer passed; all six Chrome preview regressions and
+all 240 tests in `make test-frontend` passed on the combined code. The original
+228-test result above describes the earlier isolated branch.
+
+The authorized rollout rebuilds only `frontend` from clean, pulled `main`, using
+the repository's fresh stable-release resolver and digest-pinned local inputs.
+Runtime image, HTTPS and served-bundle verification are recorded in task #69
+after the rollout; generated local pins remain ignored.

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<PhotoPreview> {
   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<PhotoPreview> {
     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),
+        ),
+      ),
+    ),
   );
 }

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.",

frontend/app/lib/l10n/ru.arb

@@ -128,6 +128,7 @@
   "kycErrorFormat": "Выберите изображение JPEG или PNG. Другие форматы не поддерживаются.",
   "kycErrorLarge": "Изображение слишком большое. Выберите файл до 10 МБ и 40 мегапикселей.",
   "kycErrorUnreadable": "Изображение повреждено или не читается. Выберите другое фото.",
+  "kycErrorPreview": "Не удалось показать фото. Попробуйте выбрать его ещё раз.",
   "kycErrorSmall": "Фото слишком маленькое. Ширина и высота должны быть не менее 600 пикселей.",
   "kycErrorIncomplete": "Выберите оба фото и подтвердите, что на них вы и ваш документ.",
   "kycErrorSession": "Сессия истекла. Войдите снова и проверьте статус верификации.",

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"

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

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<void> 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<void> decoded(WidgetTester tester, {bool succeeds = true}) async {
+  final providers = tester
+      .widgetList<Image>(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<void>();
+      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<RawImage>(find.byType(RawImage));
+      expect(raw.image, isNotNull);
+      expect(raw.image!.width, lessThanOrEqualTo(1000));
+      expect(
+        tester.widget<Image>(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<Image>(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<Image>(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<Image>(find.byType(Image)).image;
+    final replacementKey = await replacement.obtainKey(
+      ImageConfiguration.empty,
+    );
+    expect(cache.statusForKey(replacementKey).keepAlive, isTrue);
+    expect(tester.widget<RawImage>(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<RawImage>(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<RawImage>(find.byType(RawImage)).image, isNotNull);
+      },
+    );
+  }
+}