Repository navigation
Support PNG and HEIC photos in the native iOS zoom gallery - #9314
Conversation
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
GitHub Actions
Inactive
github-actions
Bot
added
the
codex-security-review-current
🔐 Codex Security Review
Review SummaryOverall Risk: LOW
Findings[LOW]
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8531e295bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (disposed || (first.isCompleted && !shouldForwardFrame())) { | ||
| info.dispose(); | ||
| return; |
There was a problem hiding this comment.
When an animated PNG has produced its first frame and the user selects another nearby photo, this branch disposes each subsequent ImageInfo but leaves the ImageStreamListener attached. Flutter's multi-frame completer therefore continues decoding every frame for each previously viewed photo still inside the five-item window, wasting CPU and battery for content that cannot be displayed. Detach or pause these listeners on deselection and reattach only when their photo becomes selected again.
AGENTS.md reference: AGENTS.md:L249-L256
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[P2] Preserve the latest APNG frame across selection changes
This also affects displayed content: a finite APNG can finish while another nearby photo is selected, then remain on an outdated frame when reopened. Selection does not forward a retained frame or restart decoding, and the native image cache suppresses another load. Could we retain the latest frame for reselection or manage playback across selection changes?
Posted by Codex (gpt-6.1-sol) on behalf of kylez
| images[value] = image | ||
| for page in pager.children.compactMap({ $0 as? NativePhotoPage }) where page.index == value { | ||
| page.setImage(image) | ||
| } |
There was a problem hiding this comment.
When an APNG advances, updateFrame refreshes only the page and cache. The zoom transition still targets the anchor UIImageView, whose image is updated only through syncSelection after the initial load or a page change, so dismissing after the animation advances zooms into the stale first frame and visibly snaps at the end. Propagate continuing frames to the anchor as well, without re-running the selection request.
Useful? React with 👍 / 👎.
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude’s code review agent — changes recommended (GitHub COMMENT per repository agent-review policy).
Reviewed: 44b3c4823f0cf1f7715f85dbf5566cb64e848162..8531e295bf009418bb695a1cde706057698a2684 (exact head). Risk: high — native/Flutter async media playback and session lifetime.
Blocking P2 — an APNG can rewind on a delayed first-image reply. mobile/ios/Runner/NativePhotoZoomCoordinator.swift:335–347 installs the pending initial image response unconditionally, even if a continuing frame has already updated images[value] and the visible page at :352–357. The Dart bridge completes the first-image result and forwards later frames as a separate imageFrame method call (mobile/lib/features/channels/native_photo_zoom_preview.dart:278–287); there is no per-index frame-order fence in the native controller. If the frame method call is handled before the pending reply callback, the later callback replaces the newer frame with the older first frame, and it remains there until another frame arrives. Existing native coverage applies the later frame after draining the original image replies, so it misses this ordering (mobile/ios/NativeControlTests/NativePhotoZoomTests.swift:35–48). Author action: fence late first-image/preview completions against a newer frame (or otherwise guarantee their application order), and add a native regression delivering a blue continuing frame before completing the held red initial image request, asserting blue remains visible. The five-item window, stale-session checks, unsupported-format fallback and listener cleanup otherwise show no separate concrete defect in the assigned format/UI and systems reviews.
Verification: clean exact-head source trace and git diff --check reported by both assigned reviewers; I independently checked the callback and frame-update code against the immutable GitHub head. Exact-head GitHub Mobile Checks, three test shards, Android, Swift Native/Release and security review are green (run 42750553085 and security run 42750552936). The independent local Flutter/native fixture did not complete: Flutter/Hermit startup timed out and the isolated checkout lacked generated iOS configuration; no booted simulator or physical-device playback was witnessed. Those are reviewer/tooling confidence gaps, not additional author defects. Verification owner: author for code and reordered native regression; reviewer/native or release QA for exact-head simulator playback and accessibility after the fix. No independent native journey is claimed. Any new head requires re-review.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: kenny lopez <[email protected]>
kazemek21
left a comment
kazemek21
left a comment
There was a problem hiding this comment.
Kyle reviewed this PR and the Nexus review-tracker notes. Approving with the non-blocking APNG feedback in the existing thread.
Posted by Codex (gpt-6.1-sol) on behalf of kylez
|
🤖 Addressed Jude’s blocking APNG rewind finding in 84e1ec2. Initial image and preview completions now preserve a newer full animation frame. The native regression delivers a blue continuing frame before releasing the held red initial reply and asserts that blue remains visible. Validation: native photo-window/stale-response/teardown fixture passed on the iPhone 18 Pro iOS 27 simulator at this commit; required full mobile push checks passed. This is native fixture coverage, not a physical-device visual review. The separate non-blocking APNG feedback remains open and is not claimed as addressed. |
github-actions
Bot
removed
the
codex-security-review-current
GitHub Actions
Inactive
github-actions
Bot
added
the
codex-security-review-current
## Summary Videos previously rendered outside photo stacks and opened only the Flutter video viewer. Message bubbles now group trailing videos and photos in their original order, with poster thumbnails or a visible play placeholder. On iOS 18+, the native zoom gallery (enabled by default) opens both standalone videos and mixed stacks with AVKit playback controls, and closing restores the selected stack item. Flutter downloads only the selected video using fresh media authentication, streams it to a bounded temporary file, and cancels/deletes it when selection changes or the viewer closes. UIKit releases the old player before that file is removed. Failed downloads and playback expose Retry. Video URLs never enter the image decoder, and photo-only actions are hidden on video pages. The existing Flutter viewers remain the fallback. Native zoom defaults on; `--dart-define=BUZZ_NATIVE_PHOTO_ZOOM=false` remains an explicit build-time opt-out. Unsupported platforms/formats, unavailable native presentation, and bridge failures use the Flutter fallback. Native video pages use Close or accessibility escape to return with zoom; the photo-specific downward drag recognizer is disabled there to avoid competing with AVKit controls. ### Related issue Stacked on #9314 (PNG/HEIC photo formats); merge that first. No matching open video-stack/zoom PR found. ### Testing - Default-on follow-up: presentation tests also cover unavailable, null, and throwing bridge responses. - `just mobile-check mobile-test`: analysis clean, 4,601 mobile tests passed, plus push-configuration and secp256k1 package tests. - Full native-enabled mobile suite: 4,626 passed, 68 skipped; rerun passed at `dc752dccaa664cb56a19cbe517a266d1407a1100`. - Mixed-stack widget journey covers order, video opening, returning to selection, and the photo-only fallback gallery. - Native bridge tests cover video snapshot presentation, selected-only dispatch, stale sessions and keeping video URLs out of image decoding. - Real HTTP download journey covers authenticated requests, Retry, byte limits, cancellation and temporary-file cleanup. - Native simulator fixture decodes a real MP4 and verifies selected-only loading, player release on paging and teardown. Removing playback cleanup makes the regression fail; restoring it passes. - No physical-device installation or end-to-end zoom recording captured. ### Widget previews Actual Flutter renders with sample video URLs and no poster metadata; these show the stack layout, not device playback. | Received, light | Sent, dark | | --- | --- | |  |  | ### Human verification pending On a normal build on iOS 18+ (no extra flag required), open a message containing photos and videos, page between them, play a video, and close. Verify playback controls work, only the selected video plays, and closing returns to the selected stack item. --------- Signed-off-by: kenny lopez <[email protected]> Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <[email protected]> * origin/main: feat(mobile/ios): show the message's own text in notifications (#9346) feat(mobile): prefill the agents you last tagged in a conversation (#9248) feat(mobile): start a Solo chat from a message or inbox row (#9251) feat(mobile): link to Builderlab web from Settings (#9300) fix(relay): bound invite preview host lookup (#9308) fix(relay): saturate filter limits instead of casting (#9230) fix(mobile): tell same-name agents apart in Solo (#9321) fix(mobile): join an open channel from the chat footer (#9253) fix(relay): fence pending subscriptions on close or replacement (#2382) fix(mobile): hide archived people and agents from choices; give them their own members section (#9320) Add video stacks and native iOS zoom playback (#9316) Support PNG and HEIC photos in the native iOS zoom gallery (#9314) fix(mobile): list known people in recipient pickers before a name is typed (#9311) feat(mobile): mark edited messages after the body (#8863) Signed-off-by: Philip Azar <[email protected]>
## Summary Photos currently filters out videos, video messages without poster metadata stay grey until playback, and the native gallery rejects downward dismissal on video pages. - Include photos and videos in the native inline/popover pickers, system picker, and recent-media grid. Mixed selections retain their order and videos use the video upload path. Remove the redundant Video menu row on iOS and Android, whose Photos picker accepts both media types; reduce their menu height to four rows. Native pickers request the current asset representation to avoid compatibility transcoding and export up to three files concurrently, preserving order and cleaning up cancelled/failed batches. - Show local stills for queued videos. For older messages without posters, serialize thumbnail generation for the four rendered stack cards, bound/cancel downloads, remove temporary videos, and save stills in the existing account/community cache. Existing poster metadata is used directly. The native viewer uses the poster while loading instead of a play-button snapshot. - Allow downward dismissal on native video pages while preserving horizontal paging/scrubbing, zoomed-photo panning, and the accessible Close button. Fire selection-click haptics as a swipe begins rather than after the page settles, with no duplicate on completion or cancellation. Page-indicator changes retain immediate feedback. Replace visible video-loading text with the existing white arc-spinner treatment, implemented natively with reduced-motion and lifecycle handling. Completed swipes release the previous video player even after UIKit removes it from the active children. Legacy videos need one initial download (up to the existing 100 MB limit) to generate their cached preview. This does not start playback. ### Related issue Follow-up to merged #9259 and #9314. Searched open and closed gallery/video issues and PRs; no duplicate fix found. ### Testing - Full `just mobile-check` and `just mobile-test` passed at `12e6ed6ae3`: 4,688 Flutter tests (71 skipped), 4 unconfigured-push tests, and 27 crypto-package tests. File-size checks passed. Regressions cover mixed image/video upload routing, queued still previews, the rendered-card poster window, authenticated/cancelled downloads, caching/cleanup, and native poster loading. - Native gallery simulator regression fixture passed: photo/video dismissal guards, real MP4 decoding, paging, selected-only playback loading, and teardown. - Swift type-check of the native picker and its dependencies passed, with existing deprecation/capture warnings. - `just ci` passed its repository-wide lint/type checks. The unrelated Rust test rebuild was stopped after consuming about 13 GB to preserve remaining disk space; only that run's generated caches were removed. Release build at `fb09f9cb8` installed in place and launched on the physical iPhone Air as **Buzz Photo Stack**, with `BUZZ_NATIVE_PHOTO_ZOOM=true`. Existing app updated in place with the same bundle and Keychain identity; no uninstall or reset. Owner tested the physical-device preview and approved readiness for review. No before/after capture was produced. The simulator evidence above is an automated native fixture, not a full Buzz device journey. Native preparation follow-up: `scripts/test-mobile-picker-preparation.sh` passed on an iPhone Air simulator, covering bounded overlap, out-of-order completion, cancellation, and failure cleanup. The updated physical Air preview launched successfully with unchanged preferences metadata. Cloud-only assets may still require preparation. Loader/haptics follow-up native fixture passed: real video readiness hides the loader, error/retry and detach stop/restart it correctly, swipe start triggers one haptic before selection changes, and completion/cancellation/duplicate-start callbacks add none. Updated release installed and launched on the physical Air; owner approved the final preview for review. Review: focused code review and native regression checks completed; physical Air preview tested by the owner, who approved readiness. P2 follow-up: video presentation uses an already decoded poster or the source snapshot immediately, then forwards a pending poster asynchronously. Late poster updates are limited to the selected native page and do not restart playback. Added pending-provider presentation coverage and native late/stale-poster checks. This follow-up has not been installed on the physical preview yet. --------- Signed-off-by: kenny lopez <[email protected]> Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <[email protected]> * origin/main: (22 commits) chore(deps): bump hickory-proto to 0.26.2 for RUSTSEC-2026-0487..0491 (#9351) feat(mobile/ios): show the message's own text in notifications (#9346) feat(mobile): prefill the agents you last tagged in a conversation (#9248) feat(mobile): start a Solo chat from a message or inbox row (#9251) feat(mobile): link to Builderlab web from Settings (#9300) fix(relay): bound invite preview host lookup (#9308) fix(relay): saturate filter limits instead of casting (#9230) fix(mobile): tell same-name agents apart in Solo (#9321) fix(mobile): join an open channel from the chat footer (#9253) fix(relay): fence pending subscriptions on close or replacement (#2382) fix(mobile): hide archived people and agents from choices; give them their own members section (#9320) Add video stacks and native iOS zoom playback (#9316) Support PNG and HEIC photos in the native iOS zoom gallery (#9314) fix(mobile): list known people in recipient pickers before a name is typed (#9311) feat(mobile): mark edited messages after the body (#8863) Add swipeable photo stacks and native iOS gallery preview (#9259) ci(docker): skip relay image publishes for main pushes outside the build context (#9310) Add manual-only relay image publishing (#9303) fix(mobile): bump Flutter to 3.47.9 for Android TLS fix (#9309) Serve video and poster previews for invite links (#9306) ... Signed-off-by: Philip Azar <[email protected]>
Summary
The opt-in iOS zoom gallery rejected PNG and HEIC/HEIF photos, so photo stacks containing those files opened the Flutter fallback. Accept these formats by MIME type or file extension, including uppercase extensions and URLs with query strings.
Preserve animated PNG playback by forwarding continuing decoded frames to the selected native page. Keep the existing five-photo load window, coalesce frames while the bridge is busy, skip offscreen frame encoding, and release listeners on eviction or dismissal. Native updates reject stale sessions, offscreen pages, malformed frames, and closed viewers.
BUZZ_NATIVE_PHOTO_ZOOMremains opt-in. iOS below 18 and unsupported formats retain the existing gallery.Related issue
Follow-up to #9259. No matching open issue or PR found; #7666 concerns desktop HEIC upload tiling, a separate path.
Testing
just mobile-install mobile-check mobile-testpassed.BUZZ_NATIVE_PHOTO_ZOOM=true: 4,623 passed, 68 skipped. Standard configuration: 4,599 passed, 92 skipped; push configuration and secp256k1 package tests also passed.scripts/test-mobile-photo-zoom.shon an iOS 27 simulator exercises the production native controller, including frame updates, stale-page rejection, malformed input and teardown.No physical-device or end-to-end zoom recording captured. This extends accepted image formats and playback without changing the gallery layout.