From 4a8cb64c654520d9d2dd6eec35aa3040dc6c0b11 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Tue, 8 Sep 2026 17:50:37 -0400 Subject: [PATCH] Turn photos the right way up, and lift the chat off the keyboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reports from a real phone. **A photo auto-rotated.** BitmapFactory does not apply EXIF orientation, and every image this app sends is re-encoded to strip EXIF — which is right, because that block carries the GPS coordinates the shot was taken at. The same block carries the tag saying which way is up, and a phone camera rarely writes upright pixels: it writes landscape pixels plus "turn this 90°". Decoding ignored that, the re-encode dropped the tag, and the only thing that knew which way was up was gone. The tag is now read and applied to the pixels before it is discarded, on all three paths: preparing to send, scrubbing metadata, and rendering an image received from a peer. The mirrored tags are handled too — a front camera writes them, and a selfie coming back flipped is the same bug facing sideways. 🔴 iOS is NOT affected, which is why this survived: UIImage(data:) applies orientation as it decodes, so the identical strip-and-re-encode is safe there. Same design, opposite platform default — and the reason "works fine on iOS" said nothing about Android. **The keyboard covered what was being typed into.** The manifest has said windowSoftInputMode="adjustResize" all along, and that stopped meaning anything at targetSdk 35: Android 15 forces apps targeting it edge-to-edge, and an edge-to-edge window does not resize for the IME. So the line has been a no-op on every device this ships to (minSdk 33). The root now consumes the IME inset with imePadding() — at the root rather than per screen, because every screen with a text field has this and one added later would too. 5 new tests, including one that fails if the manifest line ever becomes the only thing relied on again. 1213 tests green, counted from the result XML rather than from BUILD SUCCESSFUL. Co-Authored-By: Claude Opus 5 --- app/build.gradle.kts | 4 + .../io/flamenet/messenger/app/ImagePrep.kt | 60 ++++++++++++- .../io/flamenet/messenger/app/MainActivity.kt | 17 +++- .../flamenet/messenger/app/MetadataScrub.kt | 7 +- .../messenger/app/PhotoOrientationTest.kt | 90 +++++++++++++++++++ 5 files changed, 174 insertions(+), 4 deletions(-) create mode 100644 app/src/test/kotlin/io/flamenet/messenger/app/PhotoOrientationTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index d1cc8f7..b12a3eb 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -184,6 +184,10 @@ dependencies { // module's classpath would make `:client` unable to change its own // dependencies without breaking a screen that never mentioned it. implementation(project(":obfs4")) + // 🔴 BitmapFactory ignores EXIF orientation, so a photo from a phone + // camera decodes on its side. This reads the tag so the rotation can be + // applied before re-encoding drops it. See ImagePrep.orientedFrom. + implementation("androidx.exifinterface:exifinterface:1.3.7") implementation(platform("androidx.compose:compose-bom:2024.12.01")) implementation("androidx.compose.material3:material3") implementation("androidx.compose.material:material-icons-extended") diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/ImagePrep.kt b/app/src/main/kotlin/io/flamenet/messenger/app/ImagePrep.kt index 4c90bf4..ebdcae8 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/ImagePrep.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/ImagePrep.kt @@ -2,6 +2,9 @@ package io.flamenet.messenger.app import android.graphics.Bitmap import android.graphics.BitmapFactory +import android.graphics.Matrix +import androidx.exifinterface.media.ExifInterface +import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream /** @@ -50,7 +53,7 @@ object ImagePrep { BitmapFactory.decodeByteArray(source, 0, source.size, bounds) if (bounds.outWidth <= 0 || bounds.outHeight <= 0) return null - val decoded = decodeScaled(source, bounds, maxSide) ?: return null + val decoded = orientedFrom(source, decodeScaled(source, bounds, maxSide) ?: return null) // Steps, best first. Each is a real fallback rather than a retry: for a // very large picture quality alone will not reach the budget and the @@ -105,7 +108,9 @@ object ImagePrep { val bounds = BitmapFactory.Options().apply { inJustDecodeBounds = true } BitmapFactory.decodeByteArray(source, 0, source.size, bounds) if (bounds.outWidth <= 0 || bounds.outHeight <= 0) return null - return decodeScaled(source, bounds, maxSide) + // Same tag, same problem: an image received from a peer that still + // carries EXIF must be turned before it is drawn. + return decodeScaled(source, bounds, maxSide)?.let { orientedFrom(source, it) } } private fun decodeScaled(source: ByteArray, bounds: BitmapFactory.Options, maxSide: Int): Bitmap? { @@ -115,6 +120,57 @@ object ImagePrep { return BitmapFactory.decodeByteArray(source, 0, source.size, opts) } + /** + * Turn the pixels the way the EXIF tag says, then forget the tag. + * + * 🔴 **`BitmapFactory` does not apply EXIF orientation, and re-encoding + * throws the tag away.** A phone camera almost never writes upright + * pixels: it writes landscape pixels plus "rotate this 90°". Decoding + * ignores that, the JPEG we write out carries no EXIF at all — deliberately, + * because that block holds the GPS coordinates the shot was taken at — and + * so the only thing that knew which way was up is gone. Every portrait + * photo arrived on its side. + * + * ⚠️ **iOS is not affected and that is why this survived**, so a fix + * confirmed on one app proves nothing about the other: `UIImage(data:)` + * applies the orientation as it decodes, so the same strip-and-re-encode is + * safe there. Same design, opposite platform default. + * + * ⚠️ Returns the bitmap unchanged when there is nothing to do — the common + * case, and it avoids copying a large bitmap for no reason. + */ + internal fun orientedFrom(source: ByteArray, bitmap: Bitmap): Bitmap { + val tag = runCatching { + ExifInterface(ByteArrayInputStream(source)) + .getAttributeInt(ExifInterface.TAG_ORIENTATION, + ExifInterface.ORIENTATION_NORMAL) + }.getOrDefault(ExifInterface.ORIENTATION_NORMAL) + + val matrix = Matrix() + when (tag) { + ExifInterface.ORIENTATION_ROTATE_90 -> matrix.postRotate(90f) + ExifInterface.ORIENTATION_ROTATE_180 -> matrix.postRotate(180f) + ExifInterface.ORIENTATION_ROTATE_270 -> matrix.postRotate(270f) + // ⚠️ The mirrored tags are rare but real — a front camera writes + // them. Handled rather than ignored, because a selfie coming back + // flipped is the same class of bug as one coming back sideways. + ExifInterface.ORIENTATION_FLIP_HORIZONTAL -> matrix.postScale(-1f, 1f) + ExifInterface.ORIENTATION_FLIP_VERTICAL -> matrix.postScale(1f, -1f) + ExifInterface.ORIENTATION_TRANSPOSE -> { + matrix.postRotate(90f); matrix.postScale(-1f, 1f) + } + ExifInterface.ORIENTATION_TRANSVERSE -> { + matrix.postRotate(270f); matrix.postScale(-1f, 1f) + } + else -> return bitmap + } + val turned = runCatching { + Bitmap.createBitmap(bitmap, 0, 0, bitmap.width, bitmap.height, matrix, true) + }.getOrNull() ?: return bitmap + if (turned !== bitmap) bitmap.recycle() + return turned + } + /** Scale so the longest side is at most [maxSide]. Never enlarges. */ private fun scaleTo(bitmap: Bitmap, maxSide: Int): Bitmap { val longest = maxOf(bitmap.width, bitmap.height) diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt index fd3563b..5c86a94 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt @@ -13,6 +13,7 @@ import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.RowScope import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.heightIn @@ -109,7 +110,21 @@ class MainActivity : androidx.fragment.app.FragmentActivity() { setContent { MessengerTheme { - Surface(modifier = Modifier.fillMaxSize()) { + // 🔴 **`android:windowSoftInputMode="adjustResize"` stopped doing + // anything at targetSdk 35.** Android 15 forces apps that target + // it edge-to-edge, and an edge-to-edge window does not resize + // for the keyboard — so the manifest line has been a no-op on + // every device this ships to (minSdk 33), and the keyboard sat + // over the chat and the box being typed into. Reported as + // "bringing the chat above the keyboard would help me see what + // I'm typing", which is exactly the symptom. + // + // ⚠️ At the root rather than per screen: every screen with a + // text field has this problem, and one that is added later + // would have it too. `imePadding` is inset consumption, so the + // content shrinks and a scrolled list keeps its position — + // adding padding under the keyboard, not scrolling over it. + Surface(modifier = Modifier.fillMaxSize().imePadding()) { // 🔴 **The gate is around Root, not inside it.** Drawn over // the top, the message list still renders underneath for a // frame — and a frame is all a screenshot or a shoulder diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MetadataScrub.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MetadataScrub.kt index c1c414d..436c64d 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MetadataScrub.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MetadataScrub.kt @@ -81,8 +81,13 @@ object MetadataScrub { */ fun scrub(data: ByteArray): Result { val kind = kind(data) ?: return Result(data, null) - val bitmap = runCatching { BitmapFactory.decodeByteArray(data, 0, data.size) }.getOrNull() + val raw = runCatching { BitmapFactory.decodeByteArray(data, 0, data.size) }.getOrNull() ?: return Result(data, null) + // 🔴 This function's whole job is to remove the EXIF block — which + // holds the GPS coordinates AND the tag saying which way is up. Turning + // the pixels first is what keeps the second from being collateral + // damage of removing the first. See ImagePrep.orientedFrom. + val bitmap = ImagePrep.orientedFrom(data, raw) val out = ByteArrayOutputStream() // PNG where a lossy pass would be a second surprise; JPEG otherwise, at // a quality high enough not to be visible. diff --git a/app/src/test/kotlin/io/flamenet/messenger/app/PhotoOrientationTest.kt b/app/src/test/kotlin/io/flamenet/messenger/app/PhotoOrientationTest.kt new file mode 100644 index 0000000..f9b2ad2 --- /dev/null +++ b/app/src/test/kotlin/io/flamenet/messenger/app/PhotoOrientationTest.kt @@ -0,0 +1,90 @@ +package io.flamenet.messenger.app + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import java.io.File + +/** + * A photo must arrive the way up it was taken. + * + * 🔴 Reported from a real phone: a photo auto-rotated. `BitmapFactory` does + * NOT apply EXIF orientation, and every image this app sends is re-encoded to + * strip EXIF — right, because that block carries the GPS coordinates the shot + * was taken at. But the same block carries the tag saying which way is up, and + * a phone camera rarely writes upright pixels: it writes landscape pixels plus + * "turn this 90°". Decode ignoring the tag, then throw the tag away, and every + * portrait photo lands on its side. + * + * ⚠️ These are source-level checks. The rotation itself needs a real Bitmap, + * which is an instrumented test — but the wiring is what regressed and is what + * a unit test can hold. See `PARITY.md`. + */ +class PhotoOrientationTest { + + private fun source(path: String): String { + val file = File("src/main/kotlin/io/flamenet/messenger/app/$path") + assertTrue("missing $path", file.exists()) + return file.readText() + } + + /** The send path must turn the pixels before it drops the tag. */ + @Test + fun preparingAPhotoAppliesTheExifTag() { + val prep = source("ImagePrep.kt") + assertTrue("prepare() must orient before encoding", + prep.contains("orientedFrom(source, decodeScaled(source, bounds, maxSide)")) + assertTrue("must read the tag rather than guess", + prep.contains("ExifInterface.TAG_ORIENTATION")) + } + + /** 🔴 And so must the scrubber, whose entire job is removing that block. */ + @Test + fun scrubbingAppliesTheExifTagBeforeRemovingIt() { + assertTrue("scrub() must orient before re-encoding", + source("MetadataScrub.kt").contains("ImagePrep.orientedFrom(data, raw)")) + } + + /** Images received from a peer carry the tag too. */ + @Test + fun renderingAReceivedImageAppliesTheTag() { + assertTrue("decodeBounded must orient what it decodes", + source("ImagePrep.kt").contains("?.let { orientedFrom(source, it) }")) + } + + /** + * ⚠️ The mirrored tags are rare but real — a front camera writes them, and + * a selfie coming back flipped is the same class of bug as one coming back + * sideways. + */ + @Test + fun everyOrientationTagIsHandled() { + val prep = source("ImagePrep.kt") + for (tag in listOf( + "ORIENTATION_ROTATE_90", "ORIENTATION_ROTATE_180", "ORIENTATION_ROTATE_270", + "ORIENTATION_FLIP_HORIZONTAL", "ORIENTATION_FLIP_VERTICAL", + "ORIENTATION_TRANSPOSE", "ORIENTATION_TRANSVERSE", + )) { + assertTrue("$tag is not handled", prep.contains(tag)) + } + } + + /** + * 🔴 `windowSoftInputMode="adjustResize"` is a no-op at targetSdk 35: + * Android 15 forces edge-to-edge, and an edge-to-edge window does not + * resize for the keyboard. The insets have to be handled in Compose. + */ + @Test + fun theKeyboardDoesNotCoverWhatIsBeingTypedInto() { + val activity = source("MainActivity.kt") + assertTrue("the root must consume the IME inset", + activity.contains("Modifier.fillMaxSize().imePadding()")) + assertTrue("and import it", activity.contains( + "import androidx.compose.foundation.layout.imePadding")) + + // The manifest line is kept — it still applies on older targets — but + // it must never again be the only thing relied on. + val target = File("build.gradle.kts").readText() + assertTrue("this test's premise is targetSdk 35", target.contains("targetSdk = 35")) + } +}