Repository navigation
Fix forwarded voice notes autoplaying audio from another chat. - #6340
Closed
bhanage-viraj wants to merge 1 commit into
Closed
bhanage-viraj wants to merge 1 commit into
bhanage-viraj wants to merge 1 commit into
Conversation
Attachments are deduplicated by content, so finishing playback in one conversation could match a stale listener in another and start the wrong next message.
Contributor
Author
|
@sashaweiss-signal This pull request is ready for review |
Contributor
|
Hi, thanks for this, and for signing the CLA. I've copied this diff into an internal PR, since there were a number of additional fixes I made on top of it. I'll ensure that this commit is merged internally with your authorship attached. Once that merges, I'll close this PR and comment with the internal hash of your commit (for when it eventually becomes public)! |
Contributor
|
This merged internally as 9dc8c90, which will become available with an upcoming release (should be in the next week). Thanks again for the contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Attachments are deduplicated by content, so finishing playback in one conversation could match a stale listener in another and start the wrong next message.
Contributor checklist
Administrative:
Commits and testing:
Description
Fixes #6339.
Problem: After forwarding a voice message from one chat to another and playing it, the app would incorrectly autoplay the next voice message from the original chat instead of staying in the chat where playback actually happened.
Root cause:
CVAudioPlayeris a single app-wide singleton, and when playback finishes it broadcasts to every registeredCVAudioPlayerListeneracross all open conversations. Signal deduplicates attachments by content, so a forwarded voice message shares its underlyingattachmentIdwith the original. The existingaudioPlayerDidFinishguard only checkedattachmentId, so a stale listener still registered from the source conversation could match on that shared id and trigger its own (unrelated) autoplay of the next message in a different chat.Fix: Added the owning interaction id of the attachment that finished playing to the
CVAudioPlayerListener.audioPlayerDidFinishcallback, and updatedCVComponentAudioAttachmentto also require that id to match its own message's id before autoplaying. This disambiguates by "which specific message finished" rather than "which attachment content is shared", so playback finishing in one conversation can no longer trigger autoplay logic captured by a listener belonging to a different conversation.Testing: