From 58e142c5d2fcd2acd069b2f5f7fdffac24550f7d Mon Sep 17 00:00:00 2001 From: AL-Session <160798022+AL-Session@users.noreply.github.com> Date: Mon, 17 Mar 2025 13:45:54 +1100 Subject: [PATCH] Fix/SES-3278 prevent multimedia send to blinded recipients and tint buttons appropriately (#952) * Initial commit * PR feedback 1 & 2 * Commit before merging dev to check if some issues still exist * All working * PR feedback: Removed initiallyEnabled flag * PR feedback * Properly using enabled state for button styling * Updated test --------- Co-authored-by: alansley Co-authored-by: ThomasSession --- .../conversation/v2/ConversationActivityV2.kt | 23 +++--- .../conversation/v2/ConversationViewModel.kt | 38 ++++++++-- .../conversation/v2/input_bar/InputBar.kt | 29 +++++--- .../v2/input_bar/InputBarButton.kt | 70 +++++++++++++------ .../v2/input_bar/InputBarEditText.kt | 7 +- .../v2/ConversationViewModelTest.kt | 3 +- 6 files changed, 115 insertions(+), 55 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt index b73b7ba6dd..d12cc78dc7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationActivityV2.kt @@ -378,10 +378,12 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate, private val glide by lazy { Glide.with(this) } private val lockViewHitMargin by lazy { toPx(40, resources) } - private val gifButton by lazy { InputBarButton(this, R.drawable.ic_gif, hasOpaqueBackground = true) } - private val documentButton by lazy { InputBarButton(this, R.drawable.ic_file, hasOpaqueBackground = true) } - private val libraryButton by lazy { InputBarButton(this, R.drawable.ic_images, hasOpaqueBackground = true) } - private val cameraButton by lazy { InputBarButton(this, R.drawable.ic_camera, hasOpaqueBackground = true) } + + private val gifButton by lazy { InputBarButton(this, R.drawable.ic_gif, hasOpaqueBackground = true) } + private val documentButton by lazy { InputBarButton(this, R.drawable.ic_file, hasOpaqueBackground = true) } + private val libraryButton by lazy { InputBarButton(this, R.drawable.ic_images, hasOpaqueBackground = true) } + private val cameraButton by lazy { InputBarButton(this, R.drawable.ic_camera, hasOpaqueBackground = true) } + private val messageToScrollTimestamp = AtomicLong(-1) private val messageToScrollAuthor = AtomicReference(null) private val firstLoad = AtomicBoolean(true) @@ -795,19 +797,15 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate, // GIF button binding.gifButtonContainer.addView(gifButton, LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) gifButton.onUp = { showGIFPicker() } - gifButton.snIsEnabled = false // Document button binding.documentButtonContainer.addView(documentButton, LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) documentButton.onUp = { showDocumentPicker() } - documentButton.snIsEnabled = false // Library button binding.libraryButtonContainer.addView(libraryButton, LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) libraryButton.onUp = { pickFromLibrary() } - libraryButton.snIsEnabled = false // Camera button binding.cameraButtonContainer.addView(cameraButton, LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT) cameraButton.onUp = { showCamera() } - cameraButton.snIsEnabled = false } // called from onCreate @@ -1017,7 +1015,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate, viewModel.uiState.collect { state -> binding.inputBar.run { isVisible = state.showInput - showMediaControls = state.enableInputMediaControls + allowAttachMultimediaButtons = state.enableAttachMediaControls } // show or hide loading indicator @@ -1163,7 +1161,7 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate, } override fun inputBarEditTextContentChanged(newContent: CharSequence) { - val inputBarText = binding.inputBar.text // TODO check if we should be referencing newContent here instead + val inputBarText = binding.inputBar.text if (textSecurePreferences.isLinkPreviewsEnabled()) { linkPreviewViewModel.onTextChanged(this, inputBarText, 0, 0) } @@ -1199,8 +1197,11 @@ class ConversationActivityV2 : ScreenLockActionBarActivity(), InputBarDelegate, animation.start() } isShowingAttachmentOptions = !isShowingAttachmentOptions + + // Note: These custom buttons exist invisibly in the layout even when the attachments bar is not + // expanded so they MUST be disabled in such circumstances. val allButtons = listOf( cameraButton, libraryButton, documentButton, gifButton ) - allButtons.forEach { it.snIsEnabled = isShowingAttachmentOptions } + allButtons.forEach { it.isEnabled = isShowingAttachmentOptions } } override fun showVoiceMessageUI() { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt index 49dd3a613d..11e43c70ec 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModel.kt @@ -289,7 +289,7 @@ class ConversationViewModel( it.copy( shouldExit = recipient == null, showInput = shouldShowInput(recipient, community, deprecationState), - enableInputMediaControls = shouldEnableInputMediaControls(recipient), + enableAttachMediaControls = shouldEnableInputMediaControls(recipient), messageRequestState = buildMessageRequestState(recipient), ) } @@ -322,13 +322,34 @@ class ConversationViewModel( * Since we haven't been approved by them, we can't send them any media, only text */ private fun shouldEnableInputMediaControls(recipient: Recipient?): Boolean { - if (recipient != null && - (recipient.is1on1 && !recipient.isLocalNumber) && - !recipient.hasApprovedMe()) { + + // Specifically disallow multimedia if we don't have a recipient to send anything to + if (recipient == null) { + Log.i("ConversationViewModel", "Will not enable media controls for a null recipient.") return false } - return true + // Specifically allow multimedia in our note-to-self + if (recipient.isLocalNumber) return true + + // To send multimedia content to other people: + // - For 1-on-1 conversations they must have approved us as a contact. + val allowedFor1on1 = recipient.is1on1 && recipient.hasApprovedMe() + + // - For groups you just have to be a member of the group. Note: `isGroupRecipient` convers both legacy and V2 groups. + val allowedForGroup = recipient.isGroupRecipient + + // - For communities you must have write access to the community + val allowedForCommunity = (recipient.isCommunityRecipient && openGroup?.canWrite == true) + + // - For blinded recipients you must be a contact of the recipient - without which you CAN + // send them SMS messages - but they will not get through if the recipient does not have + // community message requests enabled. Being a "contact recipient" implies + // `!recipient.blocksCommunityMessageRequests` in this case. + val allowedForBlindedCommunityRecipient = recipient.isCommunityInboxRecipient && recipient.isContactRecipient + + // If any of the above are true we allow sending multimedia files - otherwise we don't + return allowedFor1on1 || allowedForGroup || allowedForCommunity || allowedForBlindedCommunityRecipient } /** @@ -1198,7 +1219,12 @@ data class ConversationUiState( val messageRequestState: MessageRequestUiState = MessageRequestUiState.Invisible, val shouldExit: Boolean = false, val showInput: Boolean = true, - val enableInputMediaControls: Boolean = true, + + // Note: These input media controls are with regard to whether the user can attach multimedia files + // or record voice messages to be sent to a recipient - they are NOT things like video or audio + // playback controls. + val enableAttachMediaControls: Boolean = true, + val showLoader: Boolean = false, ) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBar.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBar.kt index de7ed96e93..f47d1273c9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBar.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBar.kt @@ -69,11 +69,12 @@ class InputBar @JvmOverloads constructor( showOrHideInputIfNeeded() } } - var showMediaControls: Boolean = true + var allowAttachMultimediaButtons: Boolean = true set(value) { field = value - showOrHideMediaControlsIfNeeded() - binding.inputBarEditText.showMediaControls = value + updateMultimediaButtonsState() + + binding.inputBarEditText.allowMultimediaInput = value } var text: String @@ -82,9 +83,17 @@ class InputBar @JvmOverloads constructor( var voiceRecorderState = VoiceRecorderState.Idle - private val attachmentsButton = InputBarButton(context, R.drawable.ic_plus).apply { contentDescription = context.getString(R.string.AccessibilityId_attachmentsButton)} - val microphoneButton = InputBarButton(context, R.drawable.ic_mic).apply { contentDescription = context.getString(R.string.AccessibilityId_voiceMessageNew)} - private val sendButton = InputBarButton(context, R.drawable.ic_arrow_up, true).apply { contentDescription = context.getString(R.string.AccessibilityId_send)} + private val attachmentsButton = InputBarButton(context, R.drawable.ic_plus).apply { + contentDescription = context.getString(R.string.AccessibilityId_attachmentsButton) + } + + val microphoneButton = InputBarButton(context, R.drawable.ic_mic).apply { + contentDescription = context.getString(R.string.AccessibilityId_voiceMessageNew) + } + + private val sendButton = InputBarButton(context, R.drawable.ic_arrow_up, isSendButton = true).apply { + contentDescription = context.getString(R.string.AccessibilityId_send) + } init { // Attachments button @@ -105,7 +114,6 @@ class InputBar @JvmOverloads constructor( // `microphoneButton.onUp` and tap the button then the logged output order is onUp and THEN onPress! microphoneButton.setOnTouchListener(object : OnTouchListener { override fun onTouch(v: View, event: MotionEvent): Boolean { - if (!microphoneButton.snIsEnabled) return true // We only handle single finger touch events so just consume the event and bail if there are more if (event.pointerCount > 1) return true @@ -254,9 +262,10 @@ class InputBar @JvmOverloads constructor( sendButton.isVisible = showInput && text.isNotEmpty() } - private fun showOrHideMediaControlsIfNeeded() { - attachmentsButton.snIsEnabled = showMediaControls - microphoneButton.snIsEnabled = showMediaControls + private fun updateMultimediaButtonsState() { + attachmentsButton.isEnabled = allowAttachMultimediaButtons + + microphoneButton.isEnabled = allowAttachMultimediaButtons } fun addTextChangedListener(listener: (String) -> Unit) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBarButton.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBarButton.kt index 38dc21fa69..861ac91da9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBarButton.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBarButton.kt @@ -30,7 +30,6 @@ class InputBarButton : RelativeLayout { @DrawableRes private var iconID = 0 private var longPressCallback: Runnable? = null private var onDownTimestamp = 0L - var snIsEnabled = true var onPress: (() -> Unit)? = null var onMove: ((MotionEvent) -> Unit)? = null var onCancel: ((MotionEvent) -> Unit)? = null @@ -44,25 +43,31 @@ class InputBarButton : RelativeLayout { private val expandedImageViewPosition by lazy { PointF(0.0f, 0.0f) } private val collapsedImageViewPosition by lazy { PointF((expandedSize - collapsedSize) / 2, (expandedSize - collapsedSize) / 2) } - private val colorID by lazy { - if (hasOpaqueBackground) { - R.attr.input_bar_button_background_opaque - } else if (isSendButton) { - R.attr.colorAccent - } else { - R.attr.input_bar_button_background - } + private val backgroundColourId by lazy { + if (hasOpaqueBackground) { + R.attr.input_bar_button_background_opaque + } else if (isSendButton) { + R.attr.colorAccent + } else { + R.attr.input_bar_button_background + } } val expandedSize by lazy { resources.getDimension(R.dimen.input_bar_button_expanded_size) } val collapsedSize by lazy { resources.getDimension(R.dimen.input_bar_button_collapsed_size) } + override fun setEnabled(enabled: Boolean) { + super.setEnabled(enabled) + + setIconTintColour() + } + private val imageViewContainer by lazy { val result = InputBarButtonImageViewContainer(context) val size = collapsedSize.toInt() result.layoutParams = LayoutParams(size, size) result.setBackgroundResource(R.drawable.input_bar_button_background) - result.mainColor = context.getColorFromAttr(colorID) + result.mainColor = context.getColorFromAttr(backgroundColourId) if (hasOpaqueBackground) { result.strokeColor = context.getColorFromAttr(R.attr.input_bar_button_background_opaque_border) } @@ -75,9 +80,6 @@ class InputBarButton : RelativeLayout { result.layoutParams = LayoutParams(size, size) result.scaleType = ImageView.ScaleType.CENTER_INSIDE result.setImageResource(iconID) - result.imageTintList = if(isSendButton) - ColorStateList.valueOf(context.getColorFromAttr(R.attr.message_sent_text_color)) - else ColorStateList.valueOf(context.getColorFromAttr(R.attr.input_bar_button_text_color)) result } @@ -85,8 +87,11 @@ class InputBarButton : RelativeLayout { constructor(context: Context, attrs: AttributeSet) : super(context, attrs) { throw IllegalAccessException("Use InputBarButton(context:iconID:) instead.") } constructor(context: Context, attrs: AttributeSet, defStyleAttr: Int) : super(context, attrs, defStyleAttr) { throw IllegalAccessException("Use InputBarButton(context:iconID:) instead.") } - constructor(context: Context, @DrawableRes iconID: Int, isSendButton: Boolean = false, - hasOpaqueBackground: Boolean = false) : super(context) { + constructor(context: Context, + @DrawableRes iconID: Int, + isSendButton: Boolean = false, + hasOpaqueBackground: Boolean = false + ) : super(context) { this.isSendButton = isSendButton this.iconID = iconID this.hasOpaqueBackground = hasOpaqueBackground @@ -102,23 +107,23 @@ class InputBarButton : RelativeLayout { imageView.layoutParams = imageViewLayoutParams gravity = Gravity.TOP or Gravity.LEFT // Intentionally not Gravity.START isHapticFeedbackEnabled = true + this.isEnabled = isSendButton // Only enable the send button by default } fun getIconID() = iconID fun expand() { - val fromColor = context.getColorFromAttr(colorID) - val toColor = context.getAccentColor() - GlowViewUtilities.animateColorChange(imageViewContainer, fromColor, toColor) + val backgroundFromColor = context.getColorFromAttr(backgroundColourId) + val backgroundToColor = context.getAccentColor() + GlowViewUtilities.animateColorChange(imageViewContainer, backgroundFromColor, backgroundToColor) imageViewContainer.animateSizeChange(R.dimen.input_bar_button_collapsed_size, R.dimen.input_bar_button_expanded_size, animationDuration) animateImageViewContainerPositionChange(collapsedImageViewPosition, expandedImageViewPosition) } fun collapse() { - val fromColor = context.getAccentColor() - val toColor = context.getColorFromAttr(colorID) - - GlowViewUtilities.animateColorChange(imageViewContainer, fromColor, toColor) + val backgroundFromColor = context.getAccentColor() + val backgroundToColor = context.getColorFromAttr(backgroundColourId) + GlowViewUtilities.animateColorChange(imageViewContainer, backgroundFromColor, backgroundToColor) imageViewContainer.animateSizeChange(R.dimen.input_bar_button_expanded_size, R.dimen.input_bar_button_collapsed_size, animationDuration) animateImageViewContainerPositionChange(expandedImageViewPosition, collapsedImageViewPosition) } @@ -134,8 +139,27 @@ class InputBarButton : RelativeLayout { animation.start() } + // Tint the button icon the appropriate colour for the user's theme + private fun setIconTintColour() { + if (isEnabled) { + imageView.imageTintList = if (isSendButton) { + ColorStateList.valueOf(context.getColorFromAttr(R.attr.message_sent_text_color)) + } else { + ColorStateList.valueOf(context.getColorFromAttr(R.attr.input_bar_button_text_color)) + } + } else { + // Use the greyed out colour from the user theme + imageView.imageTintList = ColorStateList.valueOf(context.getColorFromAttr(R.attr.disabled)) + } + } + + override fun onTouchEvent(event: MotionEvent): Boolean { - if (!snIsEnabled) { return false } + // Ensure disabled buttons don't respond to events. + // Caution: We MUST return false here to propagate the event through to any other + // clickable elements such as avatar icons or media elements we might want to click on. + if (!this.isEnabled) return false + when (event.action) { MotionEvent.ACTION_DOWN -> onDown(event) MotionEvent.ACTION_MOVE -> onMove(event) diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBarEditText.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBarEditText.kt index 449fe8cfd4..161768ed6f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBarEditText.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/v2/input_bar/InputBarEditText.kt @@ -20,7 +20,7 @@ class InputBarEditText : AppCompatEditText { private val screenWidth get() = Resources.getSystem().displayMetrics.widthPixels var delegate: InputBarEditTextDelegate? = null - var showMediaControls: Boolean = true + var allowMultimediaInput: Boolean = true private val snMinHeight = toPx(40.0f, resources) private val snMaxHeight = toPx(80.0f, resources) @@ -49,7 +49,7 @@ class InputBarEditText : AppCompatEditText { override fun onCreateInputConnection(editorInfo: EditorInfo): InputConnection? { val ic = super.onCreateInputConnection(editorInfo) ?: return null EditorInfoCompat.setContentMimeTypes(editorInfo, - if (showMediaControls) arrayOf("image/png", "image/gif", "image/jpg") else null + if (allowMultimediaInput) arrayOf("image/png", "image/gif", "image/jpg") else null ) val callback = @@ -69,7 +69,7 @@ class InputBarEditText : AppCompatEditText { // read and display inputContentInfo asynchronously. delegate?.commitInputContent(inputContentInfo.contentUri) - true // return true if succeeded + true // return true if succeeded } return InputConnectionCompat.createWrapper(ic, editorInfo, callback) } @@ -77,7 +77,6 @@ class InputBarEditText : AppCompatEditText { } interface InputBarEditTextDelegate { - fun inputBarEditTextContentChanged(text: CharSequence) fun inputBarEditTextHeightChanged(newValue: Int) fun commitInputContent(contentUri: Uri) diff --git a/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt index 50aa48c6d8..803840d21c 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/conversation/v2/ConversationViewModelTest.kt @@ -63,7 +63,8 @@ class ConversationViewModelTest: BaseViewModelTest() { on { deprecatedTime } doReturn MutableStateFlow(ZonedDateTime.now()) }, expiredGroupManager = mock(), - usernameUtils = mock() + usernameUtils = mock(), + context = mock() ) }