feat: the status bar shows this app's mark, not a framework glyph
Both builders in PeriodNotifier called setSmallIcon(android.R.drawable.ic_dialog_info). Android masks a small icon to a white silhouette taken from its alpha channel, so what every reminder this app has ever sent put in the status bar was that framework asset's outline. ic_notification is redrawn rather than copied from ic_launcher_monochrome, and the difference is the point: that file is a 108dp launcher canvas whose shape sits in the upper safe zone because a launcher crops and masks it. Copying its geometry would have produced a small mark floating above centre. This is a 24dp canvas the ring nearly fills, opaque white throughout, because the system discards colour and supplies its own. Both call sites, not one. The public builder is what a locked screen renders and is the one that matters most here. ## Two tests, because "which resource id" is not the whole claim bothTheLockScreenAndTheShadeShowThisAppsOwnMark reads the posted Notification rather than the source, and checks the public version separately — a change made by half is the likely mistake and it fails silently on the surface this product is most careful about. theStatusBarMarkRendersAsAReadableSilhouette renders the vector and measures alpha coverage. Two failures look identical in source and completely different in the status bar: a vector that draws nothing, and one that draws a filled shape. Neither is caught by asserting a resource id. Proved: reverting only the public builder fails exactly one test, naming that builder. prove-guard exit 0. ## NotificationPrivacyTest could never run on minSdk Found while satisfying this issue's own verify line. GrantPermissionRule asked for POST_NOTIFICATIONS unconditionally, and that permission arrived in API 33 — so on PeriodMinSdk26 every test in the class errored with "Failed to grant permissions" before reaching an assertion, for a reason unrelated to what it tests. That is how it stayed unnoticed: it is the only emulator where it fails, and a green run on a modern image looks like a green run. The class guards what a LOCKED SCREEN shows. "Passes on the newest device" was never the claim worth having. The rule is conditional now, and below 33 no permission is needed to post at all, so a no-op rule is correct rather than a workaround. All six tests now pass on PeriodMinSdk26 — the first time this file has run there. ## Not verified The API 36 instrumented run. That emulator repeatedly dies the moment Gradle starts on this machine today, across three attempts and after freeing memory; it ran the whole app-lock UI verification earlier in the same session, so this is resource contention rather than a defect. The SECURITY_CHECKLIST row covers it, and both new assertions are resource-id and render checks whose substance does not vary by API level. closes #43 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
null
parent
255808f2fc
commit
caf1244cf1
|
|
@ -465,6 +465,7 @@ tasks.register("checkNoHealthLogging") {
|
|||
val themedResourceRoots: List<String> = listOf(
|
||||
"app/src/main/res",
|
||||
"core/designsystem/src/main/res",
|
||||
"core/notifications/src/main/res",
|
||||
)
|
||||
|
||||
/**
|
||||
|
|
@ -479,6 +480,9 @@ val themedDrawableExemptions: Map<String, String> = mapOf(
|
|||
"ic_launcher_monochrome" to
|
||||
"themed monochrome vector — the launcher tints it from the system palette, " +
|
||||
"so a night copy would be a second source of truth for one shape",
|
||||
"ic_notification" to
|
||||
"alpha-only status bar mark — Android masks a small icon to a silhouette " +
|
||||
"and supplies the colour itself, so a dark variant would never be drawn",
|
||||
)
|
||||
|
||||
tasks.register("checkThemedDrawables") {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ package dev.privacyllc.period.core.notifications
|
|||
|
||||
import android.app.Notification
|
||||
import android.app.NotificationManager
|
||||
import android.os.Build
|
||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||
import androidx.test.rule.GrantPermissionRule
|
||||
import androidx.test.platform.app.InstrumentationRegistry
|
||||
|
|
@ -12,6 +13,7 @@ import org.junit.Assert.assertNotNull
|
|||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.rules.TestRule
|
||||
import org.junit.runner.RunWith
|
||||
|
||||
/**
|
||||
|
|
@ -32,10 +34,30 @@ class NotificationPrivacyTest {
|
|||
* The test APK is its own package, so the app's grant does not cover it.
|
||||
* Granted here rather than with `adb shell pm grant` out of band, because a
|
||||
* test that only passes after a manual command is a test nobody can re-run.
|
||||
*
|
||||
* **Conditional, and it has to be.** `POST_NOTIFICATIONS` arrived in API 33.
|
||||
* On anything older the permission does not exist, so `GrantPermissionRule`
|
||||
* fails with "Failed to grant permissions" *before the first assertion runs*
|
||||
* — and every test in this class errors out with a message that says nothing
|
||||
* about notifications.
|
||||
*
|
||||
* That is how this file came to be un-runnable on `PeriodMinSdk26` without
|
||||
* anybody noticing: it is the only emulator where it fails, it fails for a
|
||||
* reason unrelated to what it tests, and a green run on a modern image looks
|
||||
* like a green run. The class guards what a **locked screen** shows, which
|
||||
* is the disclosure this product is most careful about, so "passes on the
|
||||
* newest device" was never the claim worth having.
|
||||
*
|
||||
* Below 33 the permission is not required to post at all, so a no-op rule is
|
||||
* correct rather than a workaround.
|
||||
*/
|
||||
@get:org.junit.Rule
|
||||
val permission: GrantPermissionRule =
|
||||
val permission: TestRule =
|
||||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
|
||||
GrantPermissionRule.grant(android.Manifest.permission.POST_NOTIFICATIONS)
|
||||
} else {
|
||||
TestRule { base, _ -> base }
|
||||
}
|
||||
|
||||
private val context = InstrumentationRegistry.getInstrumentation().targetContext
|
||||
private val manager = context.getSystemService(NotificationManager::class.java)
|
||||
|
|
@ -104,6 +126,95 @@ class NotificationPrivacyTest {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Both builders carry the app's own mark, not a framework asset.
|
||||
*
|
||||
* Android masks a small icon to a white silhouette taken from its alpha
|
||||
* channel, so the old `android.R.drawable.ic_dialog_info` reached the status
|
||||
* bar as *that* asset's outline — a generic glyph, on every reminder this
|
||||
* app has ever sent.
|
||||
*
|
||||
* Asserted on the posted `Notification` rather than by reading the source,
|
||||
* and on **both** versions: the public one is what a locked screen renders,
|
||||
* which is the surface this product is most careful about, and it is built
|
||||
* by a separate call that is easy to update by half.
|
||||
*/
|
||||
@Test
|
||||
fun bothTheLockScreenAndTheShadeShowThisAppsOwnMark() {
|
||||
val expected = R.drawable.ic_notification
|
||||
|
||||
NotificationPrivacy.entries.forEach { mode ->
|
||||
manager.cancelAll()
|
||||
val text = NotificationCopy.textFor(
|
||||
ReminderKind.PERIOD_APPROACHING, mode, daysUntil = 2,
|
||||
)
|
||||
notifier.notify(text, mode, contentIntent = null)
|
||||
|
||||
val posted = postedNotification()
|
||||
assertNotNull("nothing was posted for $mode", posted)
|
||||
|
||||
// `Notification.icon` is deprecated and is also the only reader that
|
||||
// works unchanged from API 26 to 36 — Icon.getResId() is not public
|
||||
// across that whole range.
|
||||
@Suppress("DEPRECATION")
|
||||
assertEquals("the shade icon is not this app's, in $mode", expected, posted!!.icon)
|
||||
|
||||
@Suppress("DEPRECATION")
|
||||
assertEquals(
|
||||
"the LOCK SCREEN icon is not this app's, in $mode — the public " +
|
||||
"version is a separate builder and is the one that matters most",
|
||||
expected,
|
||||
posted.publicVersion?.icon,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The mark is a silhouette that actually has a silhouette.
|
||||
*
|
||||
* Android throws the icon's colour away and tints its **alpha**, so two
|
||||
* failures look identical in source and completely different in the status
|
||||
* bar: a vector that renders nothing (invisible icon), and one that renders
|
||||
* a filled shape (a blob). Neither is caught by asserting which resource id
|
||||
* was used.
|
||||
*
|
||||
* Rendered at the size it is actually drawn and measured. A ring with one
|
||||
* dot covers roughly a fifth of its box; the bounds are wide enough not to
|
||||
* be a pixel-comparison test, and narrow enough to fail on both mistakes.
|
||||
*/
|
||||
@Test
|
||||
fun theStatusBarMarkRendersAsAReadableSilhouette() {
|
||||
val size = 24 * context.resources.displayMetrics.density.toInt()
|
||||
val drawable = requireNotNull(
|
||||
androidx.core.content.ContextCompat.getDrawable(context, R.drawable.ic_notification),
|
||||
) { "ic_notification did not inflate" }
|
||||
|
||||
val bitmap = android.graphics.Bitmap.createBitmap(
|
||||
size, size, android.graphics.Bitmap.Config.ARGB_8888,
|
||||
)
|
||||
drawable.setBounds(0, 0, size, size)
|
||||
drawable.draw(android.graphics.Canvas(bitmap))
|
||||
|
||||
var opaque = 0
|
||||
for (x in 0 until size) {
|
||||
for (y in 0 until size) {
|
||||
if (android.graphics.Color.alpha(bitmap.getPixel(x, y)) > 0) opaque++
|
||||
}
|
||||
}
|
||||
val coverage = opaque.toDouble() / (size * size)
|
||||
|
||||
assertTrue(
|
||||
"the icon rendered almost nothing (coverage $coverage) — it would be " +
|
||||
"invisible in the status bar",
|
||||
coverage > 0.08,
|
||||
)
|
||||
assertTrue(
|
||||
"the icon rendered as a near-solid block (coverage $coverage) — a " +
|
||||
"small icon is masked to its alpha, so a filled shape becomes a blob",
|
||||
coverage < 0.45,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun directModeIsTheOnlyOneMarkedPublic() {
|
||||
val text = NotificationCopy.textFor(
|
||||
|
|
|
|||
|
|
@ -99,14 +99,14 @@ class PeriodNotifier(private val context: Context) {
|
|||
// The public version is a whole second notification, and it is what a
|
||||
// locked screen renders. Built first so it cannot be forgotten.
|
||||
val public = NotificationCompat.Builder(context, channelId(privacy))
|
||||
.setSmallIcon(android.R.drawable.ic_dialog_info)
|
||||
.setSmallIcon(R.drawable.ic_notification)
|
||||
.setContentTitle(text.publicTitle)
|
||||
.apply { text.publicBody?.let { setContentText(it) } }
|
||||
.setVisibility(NotificationCompat.VISIBILITY_PUBLIC)
|
||||
.build()
|
||||
|
||||
val notification = NotificationCompat.Builder(context, channelId(privacy))
|
||||
.setSmallIcon(android.R.drawable.ic_dialog_info)
|
||||
.setSmallIcon(R.drawable.ic_notification)
|
||||
.setContentTitle(text.privateTitle)
|
||||
.setContentText(text.privateBody)
|
||||
.setStyle(NotificationCompat.BigTextStyle().bigText(text.privateBody))
|
||||
|
|
|
|||
|
|
@ -0,0 +1,43 @@
|
|||
<?xml version="1.0" encoding="utf-8"?>
|
||||
<!--
|
||||
The status bar mark: an open ring with one offset dot, the same abstract cycle
|
||||
as the themed launcher layer in app/src/main/res/drawable/ic_launcher_monochrome.xml.
|
||||
|
||||
REDRAWN rather than copied, and the difference matters. That file is a 108dp
|
||||
launcher canvas whose shape sits in the upper safe zone, because a launcher
|
||||
crops and masks it. A notification icon is a 24dp canvas the graphic nearly
|
||||
fills, so copying the launcher geometry would have produced a small mark
|
||||
floating above centre in the status bar.
|
||||
|
||||
ALPHA ONLY. Android masks a notification's small icon to a white silhouette
|
||||
taken from its alpha channel, which is the whole defect this replaces: the old
|
||||
android.R.drawable.ic_dialog_info is a full-colour framework asset, so what
|
||||
every reminder has ever shown is that asset's silhouette rather than anything
|
||||
anybody chose. Everything here is opaque white for that reason — the system
|
||||
supplies the colour, and any colour written here is discarded.
|
||||
|
||||
It also has to survive being tiny and being the app's identity on a lock
|
||||
screen. A ring with one dot still reads at 24dp in a single colour; the real
|
||||
emblem — calendar, shield, padlock, crescent, botanicals, overlapping — would
|
||||
flatten into a blob, which is the same reasoning the monochrome launcher layer
|
||||
records.
|
||||
-->
|
||||
<vector xmlns:android="http://schemas.android.com/apk/res/android"
|
||||
android:width="24dp"
|
||||
android:height="24dp"
|
||||
android:viewportWidth="24"
|
||||
android:viewportHeight="24">
|
||||
|
||||
<!-- The cycle: an open ring, centred, stroked rather than filled. -->
|
||||
<path
|
||||
android:pathData="M12,12 m-8,0 a8,8 0 1,1 16,0 a8,8 0 1,1 -16,0"
|
||||
android:strokeColor="#FFFFFFFF"
|
||||
android:strokeWidth="2"
|
||||
android:strokeLineCap="round"
|
||||
android:fillColor="#00000000" />
|
||||
|
||||
<!-- Progression: one dot on the ring, at the top. -->
|
||||
<path
|
||||
android:pathData="M12,4 m-2.25,0 a2.25,2.25 0 1,1 4.5,0 a2.25,2.25 0 1,1 -4.5,0"
|
||||
android:fillColor="#FFFFFFFF" />
|
||||
</vector>
|
||||
Loading…
Reference in New Issue