diff --git a/crates/matrix-sdk-ui/src/timeline/controller/metadata.rs b/crates/matrix-sdk-ui/src/timeline/controller/metadata.rs index 6b83b635b..7b596469a 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/metadata.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/metadata.rs @@ -38,6 +38,7 @@ use super::{ }; use crate::{ timeline::{ + controller::TimelineFocusKind, event_item::{ extract_bundled_edit_event_json, extract_poll_edit_content, extract_room_msg_edit_content, @@ -316,6 +317,7 @@ impl TimelineMetadata { raw_event: &Raw, bundled_edit_encryption_info: Option>, timeline_items: &Vector>, + timeline_focus: &TimelineFocusKind, ) -> (Option, Option) { if let AnySyncTimelineEvent::MessageLike(ev) = event { if let Some(content) = ev.original_content() { @@ -325,7 +327,12 @@ impl TimelineMetadata { relations: ev.relations(), bundled_edit_encryption_info, }); - return self.process_content_relations(&content, remote_ctx, timeline_items); + return self.process_content_relations( + &content, + remote_ctx, + timeline_items, + timeline_focus, + ); } } (None, None) @@ -341,12 +348,14 @@ impl TimelineMetadata { content: &AnyMessageLikeEventContent, remote_ctx: Option>, timeline_items: &Vector>, + timeline_focus: &TimelineFocusKind, ) -> (Option, Option) { match content { AnyMessageLikeEventContent::Sticker(content) => { let (in_reply_to, thread_root) = Self::extract_reply_and_thread_root( content.relates_to.clone().and_then(|rel| rel.try_into().ok()), timeline_items, + timeline_focus, ); if let Some(event_id) = remote_ctx.map(|ctx| ctx.event_id) { @@ -359,8 +368,11 @@ impl TimelineMetadata { AnyMessageLikeEventContent::UnstablePollStart(UnstablePollStartEventContent::New( c, )) => { - let (in_reply_to, thread_root) = - Self::extract_reply_and_thread_root(c.relates_to.clone(), timeline_items); + let (in_reply_to, thread_root) = Self::extract_reply_and_thread_root( + c.relates_to.clone(), + timeline_items, + timeline_focus, + ); // Record the bundled edit in the aggregations set, if any. if let Some(ctx) = remote_ctx { @@ -398,6 +410,7 @@ impl TimelineMetadata { let (in_reply_to, thread_root) = Self::extract_reply_and_thread_root( msg.relates_to.clone().and_then(|rel| rel.try_into().ok()), timeline_items, + timeline_focus, ); // Record the bundled edit in the aggregations set, if any. @@ -441,6 +454,7 @@ impl TimelineMetadata { fn extract_reply_and_thread_root( relates_to: Option, timeline_items: &Vector>, + timeline_focus: &TimelineFocusKind, ) -> (Option, Option) { let mut thread_root = None; @@ -450,9 +464,25 @@ impl TimelineMetadata { } RelationWithoutReplacement::Thread(thread) => { thread_root = Some(thread.event_id); - thread - .in_reply_to - .map(|in_reply_to| InReplyToDetails::new(in_reply_to.event_id, timeline_items)) + + if matches!(timeline_focus, TimelineFocusKind::Thread { .. }) + && thread.is_falling_back + { + // In general, a threaded event is marked as a response to the previous message + // in the thread, to maintain backwards compatibility with clients not + // supporting threads. + // + // But we can have actual replies to other in-thread events. The + // `is_falling_back` bool helps distinguishing both use cases. + // + // If this timeline is thread-focused, we only mark non-falling-back replies as + // actual in-thread replies. + None + } else { + thread.in_reply_to.map(|in_reply_to| { + InReplyToDetails::new(in_reply_to.event_id, timeline_items) + }) + } } _ => None, }); diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state.rs b/crates/matrix-sdk-ui/src/timeline/controller/state.rs index b901fb205..c256819e3 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state.rs @@ -165,7 +165,7 @@ impl TimelineState { let mut date_divider_adjuster = DateDividerAdjuster::new(date_divider_mode); let (in_reply_to, thread_root) = - txn.meta.process_content_relations(&content, None, &txn.items); + txn.meta.process_content_relations(&content, None, &txn.items, &txn.timeline_focus); // TODO merge with other should_add, one way or another? let should_add_new_items = match &txn.timeline_focus { diff --git a/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs b/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs index e7b02fd6e..b78555a1b 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/state_transaction.rs @@ -626,6 +626,7 @@ impl<'a> TimelineStateTransaction<'a> { &raw, bundled_edit_encryption_info, &self.items, + &self.timeline_focus, ); let should_add = self.should_add_event_item( diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs index c99262cad..03628c1df 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs @@ -102,7 +102,7 @@ async fn test_thread_backpagination() { factory .text_msg("Threaded event 4") .event_id(event_id!("$4")) - .in_thread(&thread_root_event_id, event_id!("$3")) + .in_thread_reply(&thread_root_event_id, event_id!("$2")) .into_raw_sync() .cast(), factory @@ -159,14 +159,17 @@ async fn test_thread_backpagination() { assert_eq!(items.len(), 2 + 1); // A date divider + the 2 events assert!(items[0].is_date_divider()); - assert_eq!( - items[1].as_event().unwrap().content().as_message().unwrap().body(), - "Threaded event 3" - ); - assert_eq!( - items[2].as_event().unwrap().content().as_message().unwrap().body(), - "Threaded event 4" - ); + let event_item = items[1].as_event().unwrap(); + assert_eq!(event_item.content().as_message().unwrap().body(), "Threaded event 3"); + // In a threaded timeline, threads aren't using the reply fallback, unless + // they're an actual reply to another thread event. + assert_matches!(event_item.content().in_reply_to(), None); + + let event_item = items[2].as_event().unwrap(); + assert_eq!(event_item.content().as_message().unwrap().body(), "Threaded event 4"); + // But this one is an actual reply to another thread event, so it has the + // replied-to event correctly set. + assert_eq!(event_item.content().in_reply_to().unwrap().event_id, event_id!("$2")); let hit_start = timeline.paginate_backwards(100).await.unwrap(); assert!(hit_start); @@ -181,12 +184,12 @@ async fn test_thread_backpagination() { assert_let!(VectorDiff::PushFront { value } = &timeline_updates[0]); let event_item = value.as_event().unwrap(); assert_eq!(event_item.event_id().unwrap(), event_id!("$2")); - assert_eq!(event_item.content().in_reply_to().unwrap().event_id, event_id!("$1")); + assert_matches!(event_item.content().in_reply_to(), None); assert_let!(VectorDiff::PushFront { value } = &timeline_updates[1]); let event_item = value.as_event().unwrap(); assert_eq!(event_item.event_id().unwrap(), event_id!("$1")); - assert_eq!(event_item.content().in_reply_to().unwrap().event_id, event_id!("$root")); + assert_matches!(event_item.content().in_reply_to(), None); assert_let!(VectorDiff::PushFront { value } = &timeline_updates[2]); assert_eq!(value.as_event().unwrap().event_id().unwrap(), event_id!("$root")); diff --git a/testing/matrix-sdk-test/src/event_factory.rs b/testing/matrix-sdk-test/src/event_factory.rs index 1631a7af8..8eb359a6a 100644 --- a/testing/matrix-sdk-test/src/event_factory.rs +++ b/testing/matrix-sdk-test/src/event_factory.rs @@ -345,14 +345,22 @@ impl EventBuilder { self } - /// Adds a thread relation to the root event, setting the latest thread - /// event id too. + /// Adds a thread relation to the root event, setting the reply fallback to + /// the latest in-thread event. pub fn in_thread(mut self, root: &EventId, latest_thread_event: &EventId) -> Self { self.content.relates_to = Some(Relation::Thread(Thread::plain(root.to_owned(), latest_thread_event.to_owned()))); self } + /// Adds a thread relation to the root event, that's a non-fallback reply to + /// another thread event. + pub fn in_thread_reply(mut self, root: &EventId, replied_to: &EventId) -> Self { + self.content.relates_to = + Some(Relation::Thread(Thread::reply(root.to_owned(), replied_to.to_owned()))); + self + } + /// Adds a replacement relation to the current event, with the new content /// passed. pub fn edit(