fix: prevent the site media picker from dropping gallery media on cancel - #25866
Open
dcalhoun wants to merge 3 commits into
Open
fix: prevent the site media picker from dropping gallery media on cancel#25866dcalhoun wants to merge 3 commits into
dcalhoun wants to merge 3 commits into
Conversation
`buttonCancelTapped` reported `initialSelection` back to the delegate
using the same method as `buttonDoneTapped`, making a cancel
indistinguishable from a confirmed selection.
That matters because `initialSelection` can be a subset of the media a
gallery block actually references. The picker computes it up front from
the local Core Data store, and the media library syncs asynchronously
after the picker is presented, so any media not yet cached resolves to
nothing. Cancelling then wrote that subset back to the block, dropping
the unresolved items from the user's gallery.
Report an empty selection on cancel instead. This restores the behaviour
already documented on `SiteMediaPickerViewControllerDelegate` ("If the
user cancels the flow, the selection is empty") and matches
`ExternalMediaPickerViewController`, which cancels the same way.
All three conformers already handle an empty selection as a cancel:
`SiteIconPickerPresenter` calls `onCompletion?(nil, nil)`,
`AztecPostViewController` inserts nothing, and `GutenbergMediaPickerHelper`
forwards to a callback guarded on `!media.isEmpty`.
This fixes the cancel case only. Confirming with "Add" has the same root
cause and still drops unsynced media: the late-arriving cells are
inserted unchecked because the selection is seeded once and never
reconciled, so `buttonDoneTapped` reports a set that omits them. That
needs the picker to reconcile pending selections as media arrives, and is
left to a follow-up.
Only the GutenbergKit editor passes a non-empty `initialSelection`, so
this is a no-op for the other pickers, including the legacy Gutenberg
Mobile editor, which already cancelled with an empty selection.
Collaborator
Generated by 🚫 Danger |
dcalhoun
force-pushed
the
fix/site-media-picker-cancel-drops-selection
branch
from
August 4, 2026 15:40
a00d600 to
cc58ec6
Compare
Contributor
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 33598 | |
| Version | PR #25866 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 4541fd3 | |
| Installation URL | 099nv1boqdvu8 |
Contributor
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 33598 | |
| Version | PR #25866 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 4541fd3 | |
| Installation URL | 4nul0p0agj5ko |
dcalhoun
force-pushed
the
fix/site-media-picker-cancel-drops-selection
branch
from
August 4, 2026 16:54
cc58ec6 to
9bb338d
Compare
Contributor
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
`cancellingReportsEmptySelection` fires the Cancel bar button's own `UIAction` rather than calling the delegate method directly, so the assertion exercises what `buttonCancelTapped` actually reports instead of restating the expected value. It reads the preselected media from the embedded `SiteMediaCollectionViewController`, which the picker adds as a child view controller, so no production declaration had to be widened. The test overrides `ContextManager.shared` for its duration. `mapMediaIdsToMedia` fetches from `ContextManager.shared.mainContext`, so the blog has to live there for the picker to preselect anything. Without the override the lookup returns nothing and the test degenerates into asserting that an empty selection stays empty, which passes against the pre-fix code too. The existing `useAsSharedInstance(untilTestFinished:)` helper takes an `XCTestCase` to hang its teardown on, which Swift Testing does not provide, so the override is scoped with `defer` here instead. The suite is `.serialized` because `ContextManager.overrideInstance` is global state. Verified to fail against the pre-fix code: restoring `didFinishWithSelection: initialSelection` in `buttonCancelTapped` fails the assertion with a non-empty selection.
dcalhoun
force-pushed
the
fix/site-media-picker-cancel-drops-selection
branch
from
August 4, 2026 17:21
9bb338d to
4541fd3
Compare
dcalhoun
marked this pull request as ready for review
August 4, 2026 17:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #25857, which fixed GutenbergKit site media selection.
Cancelling the media picker dropped media from a gallery (experimental block editor only).
buttonCancelTappedreportedinitialSelectionback through the same delegate method asbuttonDoneTapped, so a cancel was indistinguishable from a confirmed selection.initialSelectioncan be a subset of the media a gallery actually references — the picker resolves it up front from the local Core Data store, and the media library syncs asynchronously after presentation, so anything not yet cached resolves to nothing. Cancelling wrote that subset back to the block, silently deleting the rest.Cancel now reports an empty selection, restoring the behaviour already documented on
SiteMediaPickerViewControllerDelegate("If the user cancels the flow, the selection is empty") and matchingExternalMediaPickerViewController. All four conformers already treat an empty selection as a cancel.Known follow-up
Confirming with Add has the same root cause and still drops unsynced media: late-arriving cells are inserted unchecked because the selection is seeded once and never reconciled, so
buttonDoneTappedreports a set that omits them. Fixing it means having the picker reconcile pending selections as media arrives, which touchesSiteMediaCollectionViewController(shared with the Media tab) andMediaPickerMenu(shared with Aztec, site icon, avatar, and stock photos), and needs a decision on preserving gallery ordering for late arrivals. Left out to keep this PR scoped.Testing instructions
This bug only affects the experimental block editor (GutenbergKit). The gallery media path runs through
PostGBKEditorViewController, which is only instantiated when thenewGutenbergremote feature flag is on. The shipping block editor (Gutenberg Mobile) and Aztec are not affected.Enable it first: Me → App Settings → Experimental Features → Experimental Block Editor.
A regression test is included, verified to fail against the pre-fix code.
To reproduce the bug manually, the gallery needs media that is not yet cached locally:
Check the block's
idsattribute before and after via Switch to code editor.Note the picker must be multi-select (a Gallery block, not a single Image block) —
initialSelectionis only applied whenallowsMultipleSelectionis true.Regression areas
GutenbergMediaPickerHelperis shared with the legacy Gutenberg Mobile editor (GutenbergViewController), so that path is worth a smoke test. It callspresentSiteMediaPickerwithoutinitialSelection, so this change is a no-op there — cancel already reported an empty selection — but this was verified by code tracing rather than by running the legacy editor.Other
SiteMediaPickerViewControllerconsumers (site icon, Aztec) also pass an emptyinitialSelectionand are unaffected.