Skip to content

feat: process GutenbergKit media uploads per the app's media settings - #23142

Open
dcalhoun wants to merge 27 commits into
trunkfrom
feat/process-gutenberg-kit-media-uploads
Open

feat: process GutenbergKit media uploads per the app's media settings#23142
dcalhoun wants to merge 27 commits into
trunkfrom
feat/process-gutenberg-kit-media-uploads

Conversation

@dcalhoun

@dcalhoun dcalhoun commented Jul 23, 2026

Copy link
Copy Markdown
Member

Description

Ref CMM-1249.

Related

Device media picked in the GutenbergKit editor currently uploads raw from the WebView straight to the WordPress REST API, bypassing the app's media settings that the legacy editor honors (image optimization/resizing, quality, EXIF location stripping, video optimization).

This PR integrates GutenbergKit's new MediaUploadDelegate (from GutenbergKit#357) so device uploads are processed natively per the app's media settings before upload, mirroring WordPress-iOS#25824. GutenbergKit runs a localhost relay server while the editor is open, hands each staged file to processFile, uploads via its default uploader, and relays WordPress's raw response to the editor verbatim — so attachment objects, media_details.sizes, blob-preview replacement, save-locking and error notices all behave exactly like a direct upload.

What GBKMediaUploadProcessor.processFile does

  • Images — optimized via the existing WPMediaUtils.getOptimizedMedia (max dimension + quality from App Settings), with GPS EXIF stripped when the "Remove location" setting is on. The reported mime type/filename is corrected to the format actually written: ImageUtils re-encodes everything except PNG — HEIC and WebP included — to JPEG, so the metadata is read off the encoded output rather than the type the client declared. When processing would be a no-op (optimization off, no strip, no rotation), the original passes through untouched — no needless lossy re-encode.
  • Sideways-captured images — physically rotated for self-hosted sites when optimization is off (WP.com rotates server-side; parity with legacy, #5737).
  • GIFs — always pass through to preserve animation.
  • Videos — transcoding runs (via the existing WPVideoUtils/m4m path) only when the "Optimize videos" setting is on, keeping the transcoded file only when it's smaller than the original. Transcodes are serialized to bound codec/memory pressure, and a codec that fails to start falls back to uploading the original rather than failing the upload. This matches Android's legacy behavior (where video optimization is opt-in), rather than iOS's always-transcode. The free-plan 5-minute duration limit is checked before any of this, as a safety net — in practice the editor rejects video on free WP.com plans against the site's allowed mime types first, so it should rarely be the thing a user hits.
  • Audio and documents on free WP.com plans are rejected up front with a localized message relayed to the editor as a notice.

The mime type a part arrives with is normalized before any of the above: parameters are stripped, casing is lowered, and text/plain — what a multipart part with no Content-Type header defaults to under RFC 7578 — falls back to the filename extension. Without that, a photo sent as image/jpeg; charset=binary or with no Content-Type at all would be routed as a non-media file and skip processing entirely.

The delegate is set unconditionally for the post/page GutenbergKit editor. If GutenbergKit's upload server fails to start, uploads degrade to the existing WebView path unchanged.

Scope of the plan check

The local file-type rejection is deliberately limited to audio and documents on free WP.com plans, where the restriction is a plan entitlement and a localized message beats the server's untranslated error. It is not applied to images or videos, and not to paid or self-hosted sites.

MimeTypes is a closed, hand-maintained table that has drifted from what WordPress accepts — it has no image/avif (core-supported since 6.5), no image/svg+xml, and no text types, and it maps self-hosted to the same document set as WP.com paid. Applied to every upload it would reject files the server would happily store (a .txt or .csv on self-hosted, an AVIF anywhere). GutenbergKit already validates against the site's real allowedMimeTypes from /wp-block-editor/v1/settings before this delegate runs, so everything else is left to that check and to the server.

Avoiding needless file copies

handlesFile is a metadata-only gate GutenbergKit consults before copying an upload to a temp file; declining makes it relay the original request body straight through. It claims a file only when processing would actually read it — video optimization on (or the free-plan duration check applying), and for images optimization on, the location strip applying to a format androidx ExifInterface can rewrite, or the self-hosted rotation fallback. A paid site with video optimization off no longer writes a second full copy of every video to the cache dir for a guaranteed passthrough.

Video optimization analytics

transcodeVideo emits MEDIA_VIDEO_OPTIMIZED, MEDIA_VIDEO_CANT_OPTIMIZE, and MEDIA_VIDEO_OPTIMIZE_ERROR with the same property shapes as the legacy VideoOptimizer (including the input_video_/output_video_ prefixes, saved_megabytes, and optimizer_lib), so GutenbergKit and legacy transcodes are directly comparable. Image optimization already inherits MEDIA_PHOTO_OPTIMIZED/MEDIA_PHOTO_OPTIMIZE_ERROR from inside getOptimizedMedia.

Note for rollout comparison: on sites with video optimization off, GutenbergKit emits no video events at all, because the transcode path is never entered. Legacy would still have entered VideoOptimizer. Lower event volume there is expected, not a regression.

WPMediaUtils change

getOptimizedMedia and fixOrientationIssue now return Uri.fromFile(...) rather than Uri.parse(path). Uri.parse reads everything after a # as a fragment and after a ? as a query, so for a file named IMG_#1.jpg the resulting getPath() was truncated to a path that does not exist and the upload sent nothing. The exposure is new here — legacy callers get their paths from MediaStore, whereas GutenbergKit names the staged file after the client-supplied multipart filename — but the fix is in the shared utility, so it applies to the media browser and OptimizeMediaUseCase too. getRealPathFromURI already handles the file:// scheme these now carry.

Testing instructions

All in the GutenbergKit editor (enable the experimental block editor for the site). Change settings between runs in App Settings → Media.

Image optimization:

  1. With "Optimize images" on, insert a photo wider than the max-size setting (e.g. > 2000px).
  • Uploaded image is resized to the configured max dimension and is a JPEG.
  1. Turn "Optimize images" off and "Remove location" off; insert a photo.
  • Uploaded bytes are identical to the original (no re-encode).

Location stripping:
3. Insert a JPEG, PNG, or WebP photo with GPS EXIF, with "Remove location" on (try both with optimization on and off).

  • The uploaded file has no GPS EXIF.

GIF:
4. Insert an animated GIF.

  • It uploads unchanged and still animates.

Video:
5. With "Optimize videos" on, insert a video wider than the width setting.

  • Uploaded video is a transcoded MP4 at the configured width.
  1. Turn "Optimize videos" off; insert a video.
  • The original video uploads.

Filenames:
7. With "Optimize images" on, rename a photo to IMG_#1.jpg and insert it. Repeat with a name containing a space and one containing a literal % (e.g. 100%_done.jpg).

  • Each uploads and is resized.

File types:
8. On a self-hosted site, insert a .txt or .csv, and an AVIF image.

  • They upload rather than being rejected locally.
  1. On a free WP.com site, insert an audio file.
  • The editor shows a localized notice that the file type is disallowed.

@dangermattic

dangermattic commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator
2 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ This PR is assigned to the milestone 27.1. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@wpmobilebot

wpmobilebot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Project dependencies changes

list
! Upgraded Dependencies
org.wordpress.gutenbergkit:android:v0.20.0-alpha.0, (changed from v0.19.0)
org.wordpress:utils:156-f375205024780f54c4b6e4e834ae1bbace69322a, (changed from 3.14.0)
tree
 +--- project :libs:editor
-|    \--- org.wordpress.gutenbergkit:android:v0.19.0
+|    \--- org.wordpress.gutenbergkit:android:v0.20.0-alpha.0
 +--- project :libs:fluxc
-|    \--- org.wordpress:utils:3.14.0
-|         +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.10 -> 1.9.24 (*)
-|         +--- org.apache.commons:commons-text:1.10.0 -> 1.15.0
-|         |    \--- org.apache.commons:commons-lang3:3.20.0
-|         +--- com.android.volley:volley:1.2.0 -> 1.2.1
-|         +--- com.google.android.material:material:1.2.1 -> 1.14.0 (*)
-|         +--- androidx.swiperefreshlayout:swiperefreshlayout:1.1.0 -> 1.2.0 (*)
-|         +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-|         +--- org.greenrobot:eventbus:3.3.1 (*)
-|         \--- androidx.core:core-ktx:1.5.0 -> 1.16.0 (*)
+|    \--- org.wordpress:utils:156-f375205024780f54c4b6e4e834ae1bbace69322a
+|         +--- org.apache.commons:commons-text:1.10.0 -> 1.15.0
+|         |    \--- org.apache.commons:commons-lang3:3.20.0
+|         +--- com.google.android.material:material:1.2.1 -> 1.14.0 (*)
+|         +--- androidx.swiperefreshlayout:swiperefreshlayout:1.1.0 -> 1.2.0 (*)
+|         +--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+|         +--- org.greenrobot:eventbus:3.3.1 (*)
+|         +--- org.greenrobot:eventbus-java:3.3.1
+|         +--- androidx.core:core:1.5.0 -> 1.16.0 (*)
+|         \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.24 -> 2.4.10 (*)
-+--- org.wordpress:utils:{strictly 3.14.0} -> 3.14.0 (*)
++--- org.wordpress:utils:{strictly 156-f375205024780f54c4b6e4e834ae1bbace69322a} -> 156-f375205024780f54c4b6e4e834ae1bbace69322a (*)
-\--- org.wordpress.gutenbergkit:android:v0.19.0 (*)
+\--- org.wordpress.gutenbergkit:android:v0.20.0-alpha.0 (*)

@wpmobilebot

wpmobilebot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

App NameWordPress Android
Build TypeDebug
Versionpr23142-3ab55ca
Build Number1498
Application IDorg.wordpress.android.prealpha
Commit3ab55ca
Installation URL0mm1ugelcsqt8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

App Icon📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

App NameJetpack Android
Build TypeDebug
Versionpr23142-3ab55ca
Build Number1498
Application IDcom.jetpack.android.prealpha
Commit3ab55ca
Installation URL3e9a2sjb4vcs8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 46.83544% with 84 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.94%. Comparing base (71dfd27) to head (3ab55ca).
⚠️ Report is 7 commits behind head on trunk.

Files with missing lines Patch % Lines
...android/ui/posts/editor/GBKMediaUploadProcessor.kt 51.03% 66 Missing and 5 partials ⚠️
...va/org/wordpress/android/util/MediaUtilsWrapper.kt 0.00% 6 Missing ⚠️
.../org/wordpress/android/ui/prefs/AppPrefsWrapper.kt 0.00% 5 Missing ⚠️
.../java/org/wordpress/android/util/WPMediaUtils.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #23142      +/-   ##
==========================================
+ Coverage   37.93%   37.94%   +0.01%     
==========================================
  Files        2347     2348       +1     
  Lines      127837   127992     +155     
  Branches    17781    17820      +39     
==========================================
+ Hits        48495    48569      +74     
- Misses      75374    75450      +76     
- Partials     3968     3973       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dcalhoun and others added 14 commits August 17, 2026 09:50
Consumes the snapshot build of wordpress-mobile/GutenbergKit#357, which
adds the MediaUploadDelegate API for host-side media upload processing.
To be swapped to a tagged release before merge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds video optimization and strip-image-location accessors to
AppPrefsWrapper, and a stripImageLocation passthrough to
MediaUtilsWrapper, so the upcoming GutenbergKit media upload processor
can read settings and strip GPS EXIF through injectable, testable
wrappers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements GutenbergKit's MediaUploadDelegate to process device media
picked in the experimental block editor before upload, honoring the
app's media settings the same way the legacy editor pipeline does:

- Images are optimized per the max size/quality settings via
  WPMediaUtils.getOptimizedMedia, with GPS EXIF stripped when the
  strip-location setting is on, and the reported mime type/extension
  corrected to the actual output format (non-PNG inputs, including
  HEIC, re-encode to JPEG).
- When processing would be a no-op, the original file passes through
  untouched, avoiding a needless lossy re-encode.
- Sideways-captured images are rotated for self-hosted sites when
  optimization is off (WP.com rotates server-side; issue #5737).
- GIFs always pass through to preserve animation.
- Videos enforce the free-plan 5-minute duration limit and transcode
  via WPVideoUtils/m4m only when the optimize-video setting is on,
  keeping the transcoded file only when smaller than the original.
  Transcodes are serialized to bound codec/memory pressure.
- File types disallowed by the site plan are rejected with a localized
  message relayed to the editor as a notice.

Nothing wires the processor yet; that lands separately.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds JVM unit tests for the processor's decision table: optimized
image output with corrected mime/extension (incl. HEIC to JPEG and PNG
passthrough), no-op short-circuit, GPS stripping onto the optimized
output and onto a copy when optimization is off, GIF passthrough,
self-hosted orientation fix, plan-disallowed type and over-limit video
rejections with localized messages, video passthrough when
optimization is disabled, and same-path optimization results treated
as unprocessed.

Also adds a File-based isProhibitedVideoDuration overload to
MediaUtilsWrapper so the processor avoids Uri.fromFile, keeping it
testable on the JVM.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sets the media upload delegate on GutenbergView so device media picked
in the experimental block editor is processed per the app's media
settings before upload. The fragment stores and forwards the delegate
following the existing setNetworkRequestListener pattern (GutenbergView
tolerates assignment before or after page load), and the activity
constructs the processor with its SiteModel and injected wrappers.

If GutenbergKit's upload server fails to start, uploads degrade to the
existing WebView path unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Consumes the PR-build snapshot of WordPress-Utils-Android#156, which
stops ImageUtils.getImageOrientation from logging a spurious
"Volume data not found" error on every optimized upload staged in the
app cache dir (GutenbergKit native media uploads). To be swapped to a
tagged release once that PR merges.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
processVideo and processImage are guard-clause decision tables where
early returns read clearer than nesting; suppress ReturnCount to match
the existing house style rather than fragment the logic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
EXIF_MIME_TYPES gated the copy-and-strip branch on formats that can
carry GPS, but androidx ExifInterface.saveAttributes() can only rewrite
JPEG, PNG, and WebP. The set omitted PNG (so location-bearing PNGs
uploaded un-stripped, a regression vs. the legacy pipeline) and included
HEIC/HEIF (where saveAttributes throws an IOException that stripLocation
swallows, uploading a still-geotagged copy while appearing to honor the
setting).

Correct the set to {JPEG, PNG, WebP} and cover the PNG-stripped and
HEIC-passthrough cases in the decision-table tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
transcodeVideo logs each caught exception via .message, so Detekt's
SwallowedException never fires — only TooGenericExceptionCaught does
(NullPointerException and Exception are both on its generic-names list).
Trim the suppression to the rule that actually triggers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
GutenbergKit validates uploads in the WebView against the site's
allowedMimeTypes (fetched from /wp-block-editor/v1/settings and cached on
disk) before the request reaches this delegate, so it rejects most
disallowed types with its own localized message first. Document that this
app-side check is a fallback for the cache-miss/undefined-settings path
and can over-reject when its static MimeTypes table is stricter than the
server's cached list.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR 357 merged as 016a00e5, and the review follow-up 561 ("harden the
native media upload server") landed on top of it. Track trunk at
16aceb17 instead of the transient PR branch build, picking up 561's
fixes: idle-based body reads so a steadily-streamed large upload is no
longer failed on total duration, rejection of auth-exempt OPTIONS
carrying a body, and cancellable reads so shutdown reaps an active
connection.

Still an untagged snapshot, so the TODO stays. v0.19.0 is tagged but
predates both 357 and 561.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GutenbergKit 561 made mediaUploadDelegate throw when assigned after the
editor page starts loading: the delegate is captured once, in
onPageStarted, and the setter now check()s !hasStartedLoading. The old
setter started the upload server on assignment and re-synced
window.GBKit into a loaded page, so a late set used to work.

setMediaUploadDelegate pushed straight into a possibly-loaded view,
which would now crash. Store the delegate only and let onCreateView be
the single place it reaches the view, moved to immediately after the
GutenbergView constructor — on the preloaded-dependencies fast path
that constructor already kicks off the load.

The ordering holds: FragmentPagerAdapter.instantiateItem defers its
transaction to finishUpdate, so the activity's setMediaUploadDelegate
call returns before onCreateView runs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GutenbergKit 561 added handlesFile, a metadata-only gate consulted
before the upload server materializes a request to a temp file.
Declining relays the original body straight to WordPress.

processFile returns Original for GIFs and non-media, so today those pay
a full byte-for-byte copy only to reach the same passthrough. Decline
them up front and skip the copy.

Disallowed types are claimed rather than declined, so processFile still
runs and throws the localized rejection. Declining would relay them to
WordPress instead, spending a whole upload on a file the site's plan
won't accept and replacing our message with the server's untranslated
one. That matters because Gutenberg's own validateMimeTypeForUser
early-returns when the cached allowedMimeTypes list is absent, leaving
this check as the only validation on a cold cache.

handlesFile is an optimization hint, not the enforcement point: it sees
only the client-supplied mime type and filename, which can disagree
with the file's bytes, so the plan check inside processFile stays
authoritative.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirrors the existing processFile cases: GIF and non-media are declined
so the temp copy is skipped, images and videos are claimed.

The disallowed-type case pairs with "disallowed file type throws with
localized message", which uses the same mime type — that test would
stop reflecting reality if handlesFile ever declined non-media
unconditionally, since processFile would no longer run for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dcalhoun
dcalhoun force-pushed the feat/process-gutenberg-kit-media-uploads branch from d70d4a0 to d6db8eb Compare August 17, 2026 13:57
dcalhoun and others added 3 commits August 17, 2026 12:06
isMimeTypeSupportedBySitePlan is an exact, case-sensitive match against a
closed allowlist, but resolveMimeType passed the multipart part's raw
Content-Type header through untouched. Two shapes that header legitimately
takes were rejected as disallowed file types:

- Parameters and casing. "image/jpeg; charset=binary" and "IMAGE/JPEG" miss
  the allowlist, so a valid photo failed with the localized "file type is not
  allowed" error instead of uploading.
- A missing header. GutenbergKit's multipart parser defaults a part with no
  Content-Type to "text/plain" (RFC 7578) and selects the file part by its
  filename parameter rather than its type, so a real image can arrive labeled
  text/plain and was rejected outright.

Strip parameters, lowercase, and treat text/plain as a placeholder alongside
application/octet-stream so it falls back to the filename extension.

Also guard the extension lookup: MimeTypeMap.getSingleton() is @nonnull on
device but null under the unit test stubs, and widening the fallback to
text/plain widened that latent NPE onto the common no-Content-Type path. An
unresolvable lookup now degrades to the declared type rather than an empty
string, which the plan check cannot reason about.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
transcodeVideo documents that any failure resolves to null so the caller
uploads the original, but composer.start() was unguarded. m4m throws
IllegalStateException when it cannot set up the codec (codec unavailable,
memory pressure, unsupported track config), and that exception escaped the
coroutine through processFile into GutenbergKit's catch-all, which relayed it
to the editor as a 500 carrying the raw, untranslated m4m message. The upload
was lost where the legacy VideoOptimizer would have succeeded with the
original file. Guard it exactly as VideoOptimizer.start() does.

Also make the transcode mutex process-wide and the output filename collision
proof, which are the same defect seen from two sides:

- A new processor is constructed for every editor fragment and
  GutenbergKitActivity declares no launchMode, so instances stack. The
  per-instance mutex therefore did not serialize anything across editors,
  which is what its own KDoc says it exists to do.
- generateTimeStampedFileName is only "wp-{currentTimeMillis}.mp4", so two
  transcodes starting in the same millisecond shared an output path.
  createTempFile takes uniqueness from the filesystem with no
  check-then-create race, matching the collision-avoiding naming used by both
  MediaUtils.getUniqueCacheFileForName and the iOS exporter.

The transcode path depends on the static WPVideoUtils.getVideoOptimizationComposer
and real m4m codecs, so it has no unit coverage here, as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d type

processedImage chose the reported mime type and extension by testing the
declared *input* mime against image/png, but ImageUtils chooses its encoder
from the extension it derives for the *output*: resizeImageAndWriteToStream
writes PNG only when that extension is literally "png", and everything else —
HEIC and WebP included — becomes JPEG.

Those two decisions can disagree. optimizeImage derives its extension via
MediaUtils.getMediaFileName, which supplies one sniffed from the file's bytes
whenever the name carries no extension of its own. An extensionless upload
whose declared Content-Type disagrees with its content therefore produced JPEG
bytes reported as image/png (or PNG bytes reported as image/jpeg), and
WordPress stores that mislabel on the attachment permanently. Both the
optimization and the rotation branch were affected, and image optimization is
on by default.

Read the format off the output file instead. ImageUtils names the output with
the same extension it used to select the encoder, so the written filename is a
faithful record of the encode decision, which the declared input mime is not.
This observes what the encoder did rather than predicting it — the same
single-source-of-truth property that makes the iOS processor robust, where the
mime type is likewise derived from the exported file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dcalhoun and others added 2 commits August 18, 2026 10:09
processVideo is entered when isVideoMimeType(resolvedMimeType) is true, but
the duration guard re-derived videoness from the staged file. Wrapped in
Uri.fromFile, contentResolver.getType() returns null (it only types content
URIs), so isVideoFile collapsed to MediaUtils.isVideo -- an extension-only
test over nine suffixes.

GutenbergKit names the staged copy "{uuid}-{filename}" from the client-supplied
filename and synthesizes no extension, and that name can legitimately lack one:
Chromium takes File.name from OpenableColumns.DISPLAY_NAME on the content://
path, and a filename="" parameter reaches the delegate as an empty string. In
those cases a declared video/mp4 skipped the check entirely, letting a free-plan
site upload a video the legacy pipeline would have rejected.

Split the shared implementation so videoness is supplied by the caller. The
File overload passes isVideoMimeType(mimeType); the Uri overload keeps
isVideoFile(uri), leaving AddLocalMediaToPostUseCase and MediaBrowserActivity
unchanged -- they pass content URIs, where the resolver types correctly, which
is why the legacy editor never had this gap.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the trunk snapshot with the tagged release and drops the TODO. The
pinned commit 16aceb17 is an ancestor of the tag, and the only difference
across the android/ module between them is the GutenbergKitVersion.kt string
bump, so this is a no-op for behavior.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dcalhoun dcalhoun added this to the 27.1 milestone Aug 18, 2026
@dcalhoun dcalhoun added the Gutenberg Editing and display of Gutenberg blocks. label Aug 18, 2026
@dcalhoun
dcalhoun marked this pull request as ready for review August 18, 2026 15:15
@dcalhoun
dcalhoun requested a review from a team as a code owner August 18, 2026 15:15
@dcalhoun
dcalhoun requested review from nbradbury and removed request for a team August 18, 2026 15:15
@nbradbury

Copy link
Copy Markdown
Contributor

@dcalhoun Claude found a few issues that may need addressing.

review-gutenbergkit-media-uploads-pr23142-2026-08-18.pdf

dcalhoun and others added 8 commits August 19, 2026 12:04
The delegate rejected any upload missing from the app's static MimeTypes
table. That table is hand-maintained and has drifted from what WordPress
accepts: it has no image/avif (core-supported since 6.5), no image/svg+xml,
and no text types, and it maps self-hosted sites to the same document set as
WP.com paid. Uploads that worked before this feature — a .txt or .csv on
self-hosted, an AVIF anywhere, an SVG with the plugin enabled — were rejected
with "This file type is not allowed".

The check could only ever produce false rejections. GutenbergKit already
validates uploads in the WebView against the site's real allowedMimeTypes
from /wp-block-editor/v1/settings, so anything reaching processFile has
passed the authoritative check.

Narrow it to the one case the app can judge better than the server: audio and
documents on a free WordPress.com plan, where the restriction is a plan
entitlement and a localized message beats the server's untranslated error.
Images and videos are left to GutenbergKit and the server. application/octet-
stream is excluded because resolveMimeType emits it for unidentifiable
uploads, and "we could not identify this file" should not be answered with
"this file type is not allowed".

The free-plan test uses SiteUtils.onFreePlan, matching what
WPMediaUtils.getSitePlanForMimeTypes uses to select WP_COM_FREE, so the gate
cannot disagree with the allowlist it guards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
handlesFile claimed every image and video unconditionally. Claiming makes
GutenbergKit write a full byte-for-byte copy of the upload to a temp file
before calling processFile, so the gate is only worth paying when processing
will actually read that file.

It often will not. processVideo returns Original immediately when video
optimization is off, and the duration check only measures on a free plan
without VideoPress — so on a paid or self-hosted site with optimization off,
a common configuration since optimization is opt-in, every video upload wrote
a second full copy of the file to the cache dir for nothing. For a 1 GB video
that is a gigabyte of pointless I/O and a plausible ENOSPC. processImage has
the same shape: with optimization off, strip off, and the site on WP.com, it
returns Original without touching the file.

Gate both branches on the decisions the two methods actually make. The image
gate keeps the self-hosted term so the issue #5737 rotation fallback still
gets its file, and only counts the location strip for formats androidx
ExifInterface can rewrite — a HEIC with strip enabled reads no file either.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
setMediaUploadDelegate only stores a field; onCreateView is the sole place it
reaches GutenbergView, because GutenbergKit captures the delegate when the
page begins loading and throws from its setter afterward. Every other hook
here pushes into the live view via gutenbergView?.setX and therefore survives
a late call, but this one cannot — arriving late is a silent no-op, and
uploads quietly fall back to GutenbergKit's unprocessed WebView path.

Log it so the failure is visible rather than invisible. Verified on device
across both deferred setup paths (private WP.com and Atomic cookie fetch,
with rotation during load): the delegate reaches the view before onCreateView
in every case and the warning does not fire, so this is insurance against a
timing we could not reproduce, not a known break.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
VideoOptimizer emits MEDIA_VIDEO_OPTIMIZED, MEDIA_VIDEO_CANT_OPTIMIZE, and
MEDIA_VIDEO_OPTIMIZE_ERROR, but transcodeVideo emitted none, so GutenbergKit
video optimization was invisible in telemetry. Image optimization is
unaffected — it inherits MEDIA_PHOTO_OPTIMIZED and MEDIA_PHOTO_OPTIMIZE_ERROR
from inside getOptimizedMedia for free. With the rollout measuring parity
against the legacy editor, that asymmetry is worth closing.

Mirror VideoOptimizer's events and property shapes, including the
input_video_/output_video_ prefixes, saved_megabytes, elapsed_time_ms,
was_npe_detected, and optimizer_lib, so the two pipelines are directly
comparable. Output properties are attached only on success, where the file
still exists and its size is meaningful.

Note the composer.start() IllegalStateException path deliberately emits
nothing, matching VideoOptimizer, which also only logs there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
processImage created a temp file in the cache dir and then copied the staged
upload into it. If the copy threw — a full disk being the likely cause — the
just-created file was abandoned there: GutenbergKit only deletes the files it
is handed back, and this one never gets returned.

Delete it on failure and rethrow, so the disk-full case does not also leak.

Also drop the test's duplicate @RunWith(MockitoJUnitRunner::class), which
BaseUnitTest already carries and JUnit inherits. Its @ExperimentalCoroutinesApi
is kept: Kotlin's opt-in requirement is not inherited by subclasses, so
removing that one fails the build under -Werror.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
getOptimizedMedia and fixOrientationIssue returned Uri.parse(path) on a bare
filesystem path. Uri.parse reads everything after a '#' as a fragment and
after a '?' as a query, so for a file named "IMG_#1.jpg" the resulting
getPath() is truncated to a path that does not exist. The upload then sends
nothing or fails outright.

The exposure is new: legacy callers get their paths from MediaStore, whereas
GutenbergKit names the staged file after the client-supplied multipart
filename, which is far less constrained. Uri.fromFile encodes the path rather
than parsing it, and getRealPathFromURI already handles the file:// scheme
these now carry — its no-scheme branch only worked by accident.

Consuming a file:// Uri means getPath() is percent-encoded, so the processor
decodes before touching the filesystem, via a wrapper method because
Uri.decode is a stub returning null under unit tests. An existence check
backstops both: an unresolvable path now degrades to a clean passthrough of
the original upload instead of a File that is not there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous snapshot predates WordPress-Utils-Android#156's second commit,
which decodes percent-encoded paths before the EXIF orientation fast path so
a "file://" argument resolves instead of falling through to the failing
MediaStore query.

Still a PR build; the TODO to restore a tagged release stands.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LongParameterList on the constructor and ReturnCount on
toDistinctExistingFile. Both are suppressed rather than restructured: the
constructor takes injected wrappers but is built per-editor with a site, so
it cannot carry @Inject (which the rule exempts), and the guard clauses read
more clearly than the expression form that would satisfy the return limit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dcalhoun

Copy link
Copy Markdown
Member Author

@nbradbury I addressed the feedback and retested. I believe this ready for another review.

@nbradbury

Copy link
Copy Markdown
Contributor

@dcalhoun I noticed that stripping location merely zeros out the GPS data. I asked Claude if it should be nulled out instead, and it discovered this:

The current code isn’t just zeroing, it’s also incomplete. ExifInterface 1.4.2 defines 33 TAG_GPS_* constants. stripLocation touches 9. The other 24 are passed through untouched, and several carry real location:

TAG_GPS_AREA_INFORMATION — free-text place name
TAG_GPS_DEST_LATITUDE / _LONGITUDE / _REF — destination coordinates
TAG_GPS_IMG_DIRECTION — compass bearing the shot was taken on
TAG_GPS_SPEED, TAG_GPS_TRACK — movement
TAG_GPS_MAP_DATUM, TAG_GPS_SATELLITES, TAG_GPS_DOP, TAG_GPS_H_POSITIONING_ERROR

So a photo whose EXIF has GPSDestLatitude populated still uploads with usable coordinates today, with “Remove location” on. Zeroing vs. removing is cosmetic next to that.

@nbradbury nbradbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dcalhoun This looks good to me and I'll approve it, but I left a comment you may want to address here or in a separate PR :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gutenberg Editing and display of Gutenberg blocks. [Type] Enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants