Release 13 bugfixes

AndroidManifest.xml

@@ -40,9 +40,6 @@
     <uses-permission android:name="android.permission.READ_PHONE_NUMBERS"/>
 
     <uses-permission android:name="android.permission.CAMERA" />
-    <uses-permission android:name="android.permission.READ_MEDIA_IMAGES" />
-    <uses-permission android:name="android.permission.READ_MEDIA_VIDEO" />
-    <uses-permission android:name="android.permission.READ_MEDIA_AUDIO" />
     <uses-permission android:name="android.permission.RECORD_AUDIO" />
     <uses-permission android:name="android.permission.CALL_PHONE" />
     <uses-permission android:name="android.permission.DOWNLOAD_WITHOUT_NOTIFICATION" />

app/src/androidTest/java/com/android/messaging/datamodel/MessageNotificationAttachmentUriTest.kt

@@ -179,12 +179,34 @@ class MessageNotificationAttachmentUriTest {
         )
     }
 
+    /**
+     * Re-minting the file per pass is what put the `SMS_DELIVER` broadcast over its deadline: every
+     * pass decoded and re-compressed the same attachment again. The file is named after the part,
+     * so a pass that has already transcoded it reuses it.
+     */
     @Test
-    fun createStyledMessage_calledTwice_doesNotReuseTheSameFile() {
+    fun createStyledMessage_calledTwiceForTheSamePart_reusesTheTranscodedFile() {
         val first = messageLineInfo(attachmentUri).createStyledMessage(SENDER).dataUri
         val second = messageLineInfo(attachmentUri).createStyledMessage(SENDER).dataUri
 
-        assertNotEquals("the same file was handed to two notifications", first, second)
+        assertNotNull("no image was attached to the notification", first)
+        assertEquals("the attachment was transcoded a second time", first, second)
+        assertEquals(
+            "reusing the transcode still left a second file behind",
+            1,
+            NotificationImageProvider.listImageFiles().size,
+        )
+    }
+
+    /** Parts must not share a file, or a conversation gets handed another one's photo. */
+    @Test
+    fun createStyledMessage_forDifferentParts_usesDifferentFiles() {
+        val first = messageLineInfo(attachmentUri, PART_ID).createStyledMessage(SENDER).dataUri
+        val second =
+            messageLineInfo(attachmentUri, OTHER_PART_ID).createStyledMessage(SENDER).dataUri
+
+        assertNotNull("no image was attached to the notification", first)
+        assertNotEquals("two parts were handed the same file", first, second)
     }
 
     /**
@@ -193,7 +215,9 @@ class MessageNotificationAttachmentUriTest {
      */
     @Test
     fun sweepNotificationImages_keepsPostedImagesAndDeletesOrphans() {
-        val orphan = imageFileOf(NotificationImageProvider.buildNotificationImageUri())
+        val orphan = imageFileOf(
+            messageLineInfo(attachmentUri, OTHER_PART_ID).createStyledMessage(SENDER).dataUri,
+        )
         val message = messageLineInfo(attachmentUri).createStyledMessage(SENDER)
         val posted = imageFileOf(message.dataUri)
         assertTrue("the platform refused to grant access to the attached image", post(message))
@@ -268,16 +292,20 @@ class MessageNotificationAttachmentUriTest {
         )
     }
 
-    private fun messageLineInfo(uri: Uri): MessageNotificationState.MessageLineInfo {
+    private fun messageLineInfo(
+        uri: Uri,
+        partId: String = PART_ID,
+    ): MessageNotificationState.MessageLineInfo {
         return MessageNotificationState.MessageLineInfo(
             // authorId, authorFullName, authorFirstName
             "author",
             "Sender",
             "Sender",
-            // text, attachmentUri, attachmentType
+            // text, attachmentUri, attachmentType, attachmentPartId
             "Check out this photo!",
             uri,
             ContentType.IMAGE_JPEG,
+            partId,
             // isManualDownloadNeeded, avatarUri, messageId, timestamp, contactUriString
             false,
             null,
@@ -297,6 +325,8 @@ class MessageNotificationAttachmentUriTest {
         const val POST_POLL_MILLIS = 50L
         const val NOTIFICATION_ID = 0x7103
         const val NOTIFICATION_TAG = "BUG-015"
+        const val PART_ID = "7"
+        const val OTHER_PART_ID = "8"
         val UNGRANTABLE_URI: Uri = Uri.parse("content://call_log/calls/1")
         val SELF: Person = Person.Builder().setName("Me").build()
         val SENDER: Person = Person.Builder().setName("Sender").setKey("author").build()

app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messagedetails/MessageDetailsCopyActionA11yTest.kt

@@ -0,0 +1,109 @@
+package com.android.messaging.ui.conversation.messagedetails
+
+import android.view.accessibility.AccessibilityNodeInfo
+import androidx.activity.ComponentActivity
+import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
+import androidx.test.platform.app.InstrumentationRegistry
+import com.android.messaging.R
+import com.android.messaging.ui.core.AppTheme
+import org.junit.Assert.assertTrue
+import org.junit.Rule
+import org.junit.Test
+
+internal class MessageDetailsCopyActionA11yTest {
+
+    @get:Rule
+    val composeTestRule = createAndroidComposeRule<ComponentActivity>()
+
+    @Test
+    fun copyActionSitsOnAScreenReaderFocusableNode() {
+        composeTestRule.setContent {
+            AppTheme {
+                MessageDetailsStatusSection(
+                    sentTimestamp = null,
+                    receivedTimestamp = RECEIVED_TIMESTAMP,
+                    onCopy = {},
+                )
+            }
+        }
+        composeTestRule.waitForIdle()
+
+        val context = InstrumentationRegistry.getInstrumentation().targetContext
+        val copyLabel = context.getString(R.string.copy_to_clipboard)
+        val receivedLabel = context.getString(R.string.message_details_received_label)
+
+        val nodesOfferingCopy = awaitNodesOfferingCopy(copyLabel = copyLabel)
+
+        assertTrue(
+            "No accessibility node exposes the \"$copyLabel\" custom action at all",
+            nodesOfferingCopy.isNotEmpty(),
+        )
+
+        val focusableNodes = nodesOfferingCopy.filter { it.isScreenReaderFocusable }
+
+        assertTrue(
+            "The \"$copyLabel\" action exists on ${nodesOfferingCopy.size} node(s) but none is " +
+                "screen reader focusable, so TalkBack never offers it: " +
+                nodesOfferingCopy.joinToString { it.describe() },
+            focusableNodes.isNotEmpty(),
+        )
+
+        val receivedValue = formatMessageDetailsTimestamp(timestampMillis = RECEIVED_TIMESTAMP)
+
+        assertTrue(
+            "The focusable node offering \"$copyLabel\" announces nothing about the row it " +
+                "copies: " + focusableNodes.joinToString { it.dumpSubtree() },
+            focusableNodes.any { node ->
+                node.subtreeText().let {
+                    it.contains(receivedLabel) && it.contains(receivedValue)
+                }
+            },
+        )
+    }
+
+    private fun AccessibilityNodeInfo.subtreeText(): String {
+        return collectNodes().joinToString(separator = " ") { node ->
+            listOfNotNull(node.text, node.contentDescription).joinToString(separator = " ")
+        }
+    }
+
+    private fun AccessibilityNodeInfo.dumpSubtree(): String {
+        return collectNodes().joinToString(separator = " / ") { it.describe() }
+    }
+
+    private fun awaitNodesOfferingCopy(copyLabel: String): List<AccessibilityNodeInfo> {
+        val uiAutomation = InstrumentationRegistry.getInstrumentation().uiAutomation
+        repeat(A11Y_TREE_POLL_ATTEMPTS) {
+            val root = uiAutomation.rootInActiveWindow
+            val matches = root?.collectNodes().orEmpty().filter { node ->
+                node.actionList.any { it.label == copyLabel }
+            }
+            if (matches.isNotEmpty()) {
+                return matches
+            }
+            Thread.sleep(A11Y_TREE_POLL_INTERVAL_MILLIS)
+        }
+        return emptyList()
+    }
+
+    private fun AccessibilityNodeInfo.collectNodes(): List<AccessibilityNodeInfo> {
+        return buildList {
+            add(this@collectNodes)
+            repeat(childCount) { index ->
+                getChild(index)?.let { addAll(it.collectNodes()) }
+            }
+        }
+    }
+
+    private fun AccessibilityNodeInfo.describe(): String {
+        return "[text=$text, contentDescription=$contentDescription, " +
+            "screenReaderFocusable=$isScreenReaderFocusable, " +
+            "actions=${actionList.mapNotNull { it.label }}]"
+    }
+
+    private companion object {
+        private const val RECEIVED_TIMESTAMP = 1_700_000_000_000L
+        private const val A11Y_TREE_POLL_ATTEMPTS = 20
+        private const val A11Y_TREE_POLL_INTERVAL_MILLIS = 250L
+    }
+}

app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messages/ui/message/rendering/ConversationMessageSenderA11yTest.kt

@@ -0,0 +1,123 @@
+package com.android.messaging.ui.conversation.messages.ui.message.rendering
+
+import android.view.accessibility.AccessibilityNodeInfo
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import com.android.messaging.R
+import com.android.messaging.ui.conversation.messages.model.message.ConversationMessageUiModel
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@RunWith(AndroidJUnit4::class)
+internal class ConversationMessageSenderA11yTest : BaseConversationMessageRenderingTest() {
+
+    @Test
+    fun incomingMessageIsAnnouncedWithItsSender() {
+        setConversationMessageContent(
+            message = message(
+                text = INCOMING_TEXT,
+                status = ConversationMessageUiModel.Status.Incoming.Complete,
+                isIncoming = true,
+                senderDisplayName = SENDER_DISPLAY_NAME,
+            ),
+            showIncomingParticipantIdentity = false,
+        )
+
+        assertAnnouncedWithTheBody(
+            body = INCOMING_TEXT,
+            announcement = string(
+                resourceId = R.string.incoming_sender_content_description,
+                SENDER_DISPLAY_NAME,
+            ),
+        )
+    }
+
+    @Test
+    fun outgoingMessageIsAnnouncedAsSentByTheUser() {
+        setConversationMessageContent(message = message(text = OUTGOING_TEXT))
+
+        assertAnnouncedWithTheBody(
+            body = OUTGOING_TEXT,
+            announcement = string(resourceId = R.string.outgoing_sender_content_description),
+        )
+    }
+
+    private fun assertAnnouncedWithTheBody(body: String, announcement: String) {
+        composeTestRule.waitForIdle()
+
+        val bubbleNodes = awaitFocusableNodesSpeaking(text = body)
+
+        assertTrue(
+            "No screen reader focusable node speaks the message body \"$body\" at all. Tree: " +
+                dumpTree(),
+            bubbleNodes.isNotEmpty(),
+        )
+
+        assertTrue(
+            "TalkBack cannot tell who sent this message: the focusable node speaking \"$body\" " +
+                "never says \"$announcement\". Nodes: " +
+                bubbleNodes.joinToString { it.dumpSubtree() },
+            bubbleNodes.any { node -> node.subtreeText().contains(announcement) },
+        )
+    }
+
+    /** The bubbles TalkBack would stop on and read the body out from. */
+    private fun awaitFocusableNodesSpeaking(text: String): List<AccessibilityNodeInfo> {
+        val uiAutomation = InstrumentationRegistry.getInstrumentation().uiAutomation
+        repeat(A11Y_TREE_POLL_ATTEMPTS) {
+            val matches = uiAutomation.rootInActiveWindow?.collectNodes().orEmpty().filter { node ->
+                node.isScreenReaderFocusable && node.subtreeText().contains(text)
+            }
+            if (matches.isNotEmpty()) {
+                return matches
+            }
+            Thread.sleep(A11Y_TREE_POLL_INTERVAL_MILLIS)
+        }
+        return emptyList()
+    }
+
+    private fun dumpTree(): String {
+        val uiAutomation = InstrumentationRegistry.getInstrumentation().uiAutomation
+        return uiAutomation.rootInActiveWindow?.dumpSubtree() ?: "<no active window>"
+    }
+
+    private fun AccessibilityNodeInfo.subtreeText(): String {
+        return collectNodes().joinToString(separator = " ") { node ->
+            listOfNotNull(node.text, node.contentDescription).joinToString(separator = " ")
+        }
+    }
+
+    private fun AccessibilityNodeInfo.dumpSubtree(): String {
+        return collectNodes().joinToString(separator = " / ") { it.describe() }
+    }
+
+    private fun AccessibilityNodeInfo.collectNodes(): List<AccessibilityNodeInfo> {
+        return buildList {
+            add(this@collectNodes)
+            repeat(childCount) { index ->
+                getChild(index)?.let { addAll(it.collectNodes()) }
+            }
+        }
+    }
+
+    private fun AccessibilityNodeInfo.describe(): String {
+        return "[text=$text, contentDescription=$contentDescription, " +
+            "screenReaderFocusable=$isScreenReaderFocusable]"
+    }
+
+    private fun string(resourceId: Int, vararg formatArgs: Any): String {
+        return InstrumentationRegistry
+            .getInstrumentation()
+            .targetContext
+            .getString(resourceId, *formatArgs)
+    }
+
+    private companion object {
+        private const val INCOMING_TEXT = "Can you review this before tonight?"
+        private const val OUTGOING_TEXT = "I am on my way."
+        private const val SENDER_DISPLAY_NAME = "Ada Lovelace"
+        private const val A11Y_TREE_POLL_ATTEMPTS = 20
+        private const val A11Y_TREE_POLL_INTERVAL_MILLIS = 250L
+    }
+}

app/src/test/java/com/android/messaging/datamodel/ActionSyncTestDataModel.java

@@ -8,9 +8,7 @@
 import com.android.messaging.datamodel.data.ConversationData;
 import com.android.messaging.datamodel.data.ConversationListData;
 import com.android.messaging.datamodel.data.DraftMessageData;
-import com.android.messaging.datamodel.data.GalleryGridItemData;
 import com.android.messaging.datamodel.data.LaunchConversationData;
-import com.android.messaging.datamodel.data.MediaPickerData;
 import com.android.messaging.util.Assert;
 
 public class ActionSyncTestDataModel extends DataModel {
@@ -37,16 +35,6 @@ public ConversationData createConversationData(final Context context,
         throw new UnsupportedOperationException();
     }
 
-    @Override
-    public MediaPickerData createMediaPickerData(final Context context) {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public GalleryGridItemData createGalleryGridItemData() {
-        throw new UnsupportedOperationException();
-    }
-
     @Override
     public LaunchConversationData createLaunchConversationData(
             final LaunchConversationData.LaunchConversationDataListener listener) {