Fix room name display issues for invited rooms and newly joined rooms

TigerInYourDream · TigerInYourDream · commit 612a10e9a156 · 2025-10-24T00:12:55.000+08:00
This PR addresses room name display issues in three scenarios: 1. Room invitations showing "Empty Room" instead of actual names 2. Tombstoned room replacements not showing correct names without restart 3. Rooms joined from other clients not displaying names correctly Root cause: The comparison logic in `update_room()` was comparing references to SDK's internal state rather than cached snapshots, causing UI updates to be skipped when room names changed. Key changes: - Use `RoomDisplayName` enum instead of `Option<String>` for type safety - Properly preserve `EmptyWas` variant to maintain previous name semantics - Implement name fallback order: canonical_alias → alt_alias → room_id - Update invited room display list when names change - Store and compare cached display names to detect all changes - Only send updates when names actually change (not on every event) Addresses all review comments from #590. Signed-off-by: Alvin <zyzzz0928@gmail.com>
diff --git a/src/home/invite_screen.rs b/src/home/invite_screen.rs
@@ -8,7 +8,7 @@ use std::ops::Deref;
 use makepad_widgets::*;
 use matrix_sdk::ruma::OwnedRoomId;
 
-use crate::{app::AppStateAction, home::rooms_list::RoomsListRef, join_leave_room_modal::{JoinLeaveModalKind, JoinLeaveRoomModalAction}, room::{BasicRoomDetails, RoomPreviewAvatar}, shared::{avatar::AvatarWidgetRefExt, popup_list::{enqueue_popup_notification, PopupItem, PopupKind}, restore_status_view::RestoreStatusViewWidgetExt}, sliding_sync::{submit_async_request, MatrixRequest}, utils::{self, room_name_or_id}};
+use crate::{app::AppStateAction, home::rooms_list::RoomsListRef, join_leave_room_modal::{JoinLeaveModalKind, JoinLeaveRoomModalAction}, room::{BasicRoomDetails, RoomPreviewAvatar}, shared::{avatar::AvatarWidgetRefExt, popup_list::{enqueue_popup_notification, PopupItem, PopupKind}, restore_status_view::RestoreStatusViewWidgetExt}, sliding_sync::{submit_async_request, MatrixRequest}, utils::{self, display_name_with_fallback, room_name_or_id}};
 
 use super::rooms_list::{InviteState, InviterInfo};
 
@@ -527,7 +527,12 @@ impl InviteScreen {
             self.info = Some(InviteDetails {
                 room_info: BasicRoomDetails {
                     room_id: room_id.clone(),
-                    room_name: invite.room_name.clone(),
+                    room_name: Some(display_name_with_fallback(
+                        &invite.room_name,
+                        invite.canonical_alias.as_ref(),
+                        &invite.alt_aliases,
+                        &invite.room_id,
+                    )),
                     room_avatar: invite.room_avatar.clone(),
                 },
                 inviter: invite.inviter_info.clone(),
diff --git a/src/home/room_preview.rs b/src/home/room_preview.rs
@@ -5,7 +5,7 @@ use crate::{
     room::RoomPreviewAvatar, shared::{
         avatar::AvatarWidgetExt,
         html_or_plaintext::HtmlOrPlaintextWidgetExt, unread_badge::UnreadBadgeWidgetExt as _,
-    }, utils::{self, relative_format}
+    }, utils::{self, display_name_with_fallback, relative_format}
 };
 
 use super::rooms_list::{InvitedRoomInfo, InviterInfo, JoinedRoomInfo, RoomsListScopeProps};
@@ -296,9 +296,13 @@ impl RoomPreviewContent {
         cx: &mut Cx,
         room_info: &JoinedRoomInfo,
     ) {
-        if let Some(ref name) = room_info.room_name {
-            self.view.label(id!(room_name)).set_text(cx, name);
-        }
+        let display_name = display_name_with_fallback(
+            &room_info.room_name,
+            room_info.canonical_alias.as_ref(),
+            &room_info.alt_aliases,
+            &room_info.room_id,
+        );
+        self.view.label(id!(room_name)).set_text(cx, &display_name);
         if let Some((ts, msg)) = room_info.latest.as_ref() {
             if let Some(human_readable_date) = relative_format(*ts) {
                 self.view
@@ -324,11 +328,13 @@ impl RoomPreviewContent {
         cx: &mut Cx,
         room_info: &InvitedRoomInfo,
     ) {
-        self.view.label(id!(room_name)).set_text(
-            cx,
-            room_info.room_name.as_deref()
-                .unwrap_or("Invite to unnamed room"),
+        let display_name = display_name_with_fallback(
+            &room_info.room_name,
+            room_info.canonical_alias.as_ref(),
+            &room_info.alt_aliases,
+            &room_info.room_id,
         );
+        self.view.label(id!(room_name)).set_text(cx, &display_name);
         // Hide the timestamp field, and use the latest message field to show the inviter.
         self.view.label(id!(timestamp)).set_text(cx, "");
         let inviter_string = match &room_info.inviter_info {
diff --git a/src/home/rooms_list.rs b/src/home/rooms_list.rs
@@ -1,12 +1,12 @@
 use std::{cell::RefCell, collections::HashMap, rc::Rc, sync::Arc};
 use crossbeam_queue::SegQueue;
 use makepad_widgets::*;
-use matrix_sdk::{ruma::{events::tag::Tags, MilliSecondsSinceUnixEpoch, OwnedRoomAliasId, OwnedRoomId, OwnedUserId}, RoomState};
+use matrix_sdk::{RoomDisplayName, ruma::{events::tag::Tags, MilliSecondsSinceUnixEpoch, OwnedRoomAliasId, OwnedRoomId, OwnedUserId}, RoomState};
 use crate::{
     app::{AppState, SelectedRoom},
     room::{room_display_filter::{RoomDisplayFilter, RoomDisplayFilterBuilder, RoomFilterCriteria, SortFn}, RoomPreviewAvatar},
     shared::{collapsible_header::{CollapsibleHeaderAction, CollapsibleHeaderWidgetRefExt, HeaderCategory}, jump_to_bottom_button::UnreadMessageCount, popup_list::{enqueue_popup_notification, PopupItem, PopupKind}, room_filter_input_bar::RoomFilterAction},
-    sliding_sync::{submit_async_request, MatrixRequest, PaginationDirection}, utils::room_name_or_id,
+    sliding_sync::{submit_async_request, MatrixRequest, PaginationDirection}, utils::display_name_with_fallback,
 };
 use super::room_preview::RoomPreviewAction;
 
@@ -132,7 +132,7 @@ pub enum RoomsListUpdate {
     /// Update the displayable name for the given room.
     UpdateRoomName {
         room_id: OwnedRoomId,
-        new_room_name: Option<String>,
+        new_room_name: RoomDisplayName,
     },
     /// Update the avatar (image) for the given room.
     UpdateRoomAvatar {
@@ -201,7 +201,7 @@ pub struct JoinedRoomInfo {
     /// The matrix ID of this room.
     pub room_id: OwnedRoomId,
     /// The displayable name of this room, if known.
-    pub room_name: Option<String>,
+    pub room_name: RoomDisplayName,
     /// The number of unread messages in this room.
     pub num_unread_messages: u64,
     /// The number of unread mentions in this room.
@@ -240,7 +240,7 @@ pub struct InvitedRoomInfo {
     /// The matrix ID of this room.
     pub room_id: OwnedRoomId,
     /// The displayable name of this room, if known.
-    pub room_name: Option<String>,
+    pub room_name: RoomDisplayName,
     /// The canonical alias for this room, if any.
     pub canonical_alias: Option<OwnedRoomAliasId>,
     /// The alternative aliases for this room, if any.
@@ -399,7 +399,6 @@ impl RoomsList {
                 RoomsListUpdate::AddJoinedRoom(joined_room) => {
                     let room_id = joined_room.room_id.clone();
                     let is_direct = joined_room.is_direct;
-                    let room_name = joined_room.room_name.clone();
                     let should_display = (self.display_filter)(&joined_room);
                     let replaced = self.all_joined_rooms.insert(room_id.clone(), joined_room);
                     if replaced.is_none() {
@@ -425,10 +424,19 @@ impl RoomsList {
                         self.displayed_invited_rooms.iter()
                             .position(|r| r == &room_id)
                             .map(|index| self.displayed_invited_rooms.remove(index));
+                        let new_room_name = self.all_joined_rooms.get(&room_id).map(|room| display_name_with_fallback(
+                            &room.room_name,
+                            room.canonical_alias.as_ref(),
+                            &room.alt_aliases,
+                            &room.room_id,
+                        ));
                         cx.widget_action(
                             self.widget_uid(),
                             &scope.path,
-                            RoomsListAction::InviteAccepted { room_id, room_name }
+                            RoomsListAction::InviteAccepted {
+                                room_id: room_id.clone(),
+                                room_name: new_room_name,
+                            }
                         );
                     }
                     self.update_status_rooms_count();
@@ -460,9 +468,11 @@ impl RoomsList {
                     }
                 }
                 RoomsListUpdate::UpdateRoomName { room_id, new_room_name } => {
+                    // Try to update joined room first
                     if let Some(room) = self.all_joined_rooms.get_mut(&room_id) {
                         let was_displayed = (self.display_filter)(room);
-                        room.room_name = new_room_name;
+                        // Keep the full RoomDisplayName to preserve EmptyWas semantics
+                        room.room_name = new_room_name.clone();
                         let should_display = (self.display_filter)(room);
                         match (was_displayed, should_display) {
                             // No need to update the displayed rooms list.
@@ -482,24 +492,53 @@ impl RoomsList {
                             // Room was not displayed but should now be displayed.
                             (false, true) => {
                                 if room.is_direct {
-                                    self.displayed_direct_rooms.push(room_id);
+                                    self.displayed_direct_rooms.push(room_id.clone());
                                 } else {
-                                    self.displayed_regular_rooms.push(room_id);
+                                    self.displayed_regular_rooms.push(room_id.clone());
                                 }
                             }
                         }
-                    } else {
-                        error!("Error: couldn't find room {room_id} to update room name");
+                    }
+                    // If not a joined room, try to update invited room
+                    else {
+                        let invited_rooms_ref = get_invited_rooms(cx);
+                        let mut invited_rooms = invited_rooms_ref.borrow_mut();
+                        if let Some(invited_room) = invited_rooms.get_mut(&room_id) {
+                            let was_displayed = (self.display_filter)(invited_room);
+                            invited_room.room_name = new_room_name;
+                            let should_display = (self.display_filter)(invited_room);
+                            match (was_displayed, should_display) {
+                                (true, true) | (false, false) => { }
+                                (true, false) => {
+                                    self.displayed_invited_rooms.iter()
+                                        .position(|r| r == &room_id)
+                                        .map(|index| self.displayed_invited_rooms.remove(index));
+                                }
+                                (false, true) => {
+                                    if !self.displayed_invited_rooms.contains(&room_id) {
+                                        self.displayed_invited_rooms.push(room_id.clone());
+                                    }
+                                }
+                            }
+                        } else {
+                            warning!("Warning: couldn't find invited room {} to update room name", room_id);
+                        }
                     }
                 }
                 RoomsListUpdate::UpdateIsDirect { room_id, is_direct } => {
                     if let Some(room) = self.all_joined_rooms.get_mut(&room_id) {
                         if room.is_direct == is_direct {
                             continue;
                         }
+                        let room_name_text = display_name_with_fallback(
+                            &room.room_name,
+                            room.canonical_alias.as_ref(),
+                            &room.alt_aliases,
+                            &room.room_id,
+                        );
                         enqueue_popup_notification(PopupItem {
                             message: format!("{} was changed from {} to {}.",
-                                room_name_or_id(room.room_name.as_ref(), &room_id),
+                                room_name_text,
                                 if room.is_direct { "direct" } else { "regular" },
                                 if is_direct { "direct" } else { "regular" }
                             ),
@@ -832,10 +871,30 @@ impl RoomsList {
     /// Returns a room's avatar and displayable name.
     pub fn get_room_avatar_and_name(&self, room_id: &OwnedRoomId) -> Option<(RoomPreviewAvatar, Option<String>)> {
         self.all_joined_rooms.get(room_id)
-            .map(|room_info| (room_info.avatar.clone(), room_info.room_name.clone()))
+            .map(|room_info| {
+                (
+                    room_info.avatar.clone(),
+                    Some(display_name_with_fallback(
+                        &room_info.room_name,
+                        room_info.canonical_alias.as_ref(),
+                        &room_info.alt_aliases,
+                        &room_info.room_id,
+                    )),
+                )
+            })
             .or_else(|| {
                 self.invited_rooms.borrow().get(room_id)
-                    .map(|room_info| (room_info.room_avatar.clone(), room_info.room_name.clone()))
+                    .map(|room_info| {
+                        (
+                            room_info.room_avatar.clone(),
+                            Some(display_name_with_fallback(
+                                &room_info.room_name,
+                                room_info.canonical_alias.as_ref(),
+                                &room_info.alt_aliases,
+                                &room_info.room_id,
+                            )),
+                        )
+                    })
             })
     }
 }
@@ -860,12 +919,22 @@ impl Widget for RoomsList {
                 let new_selected_room = if let Some(jr) = self.all_joined_rooms.get(&clicked_room_id) {
                     SelectedRoom::JoinedRoom {
                         room_id: jr.room_id.clone().into(),
-                        room_name: jr.room_name.clone(),
+                        room_name: Some(display_name_with_fallback(
+                            &jr.room_name,
+                            jr.canonical_alias.as_ref(),
+                            &jr.alt_aliases,
+                            &jr.room_id,
+                        )),
                     }
                 } else if let Some(ir) = self.invited_rooms.borrow().get(&clicked_room_id) {
                     SelectedRoom::InvitedRoom {
                         room_id: ir.room_id.to_owned().into(),
-                        room_name: ir.room_name.clone(),
+                        room_name: Some(display_name_with_fallback(
+                            &ir.room_name,
+                            ir.canonical_alias.as_ref(),
+                            &ir.alt_aliases,
+                            &ir.room_id,
+                        )),
                     }
                 } else {
                     error!("BUG: couldn't find clicked room details for room {clicked_room_id}");
diff --git a/src/room/room_display_filter.rs b/src/room/room_display_filter.rs
@@ -7,7 +7,7 @@ use matrix_sdk::ruma::{
     OwnedRoomAliasId, RoomAliasId, RoomId,
 };
 
-use crate::home::rooms_list::{InvitedRoomInfo, JoinedRoomInfo};
+use crate::{home::rooms_list::{InvitedRoomInfo, JoinedRoomInfo}, utils::display_name_with_fallback};
 
 static EMPTY_TAGS: Tags = BTreeMap::new();
 
@@ -29,7 +29,12 @@ impl FilterableRoom for JoinedRoomInfo {
     }
 
     fn room_name(&self) -> Cow<'_, str> {
-        self.room_name.as_deref().map(Into::into).unwrap_or_default()
+        Cow::Owned(display_name_with_fallback(
+            &self.room_name,
+            self.canonical_alias.as_ref(),
+            &self.alt_aliases,
+            &self.room_id,
+        ))
     }
 
     fn unread_mentions(&self) -> u64 {
@@ -63,7 +68,12 @@ impl FilterableRoom for InvitedRoomInfo {
     }
 
     fn room_name(&self) -> Cow<'_, str> {
-        self.room_name.as_deref().map(Into::into).unwrap_or_default()
+        Cow::Owned(display_name_with_fallback(
+            &self.room_name,
+            self.canonical_alias.as_ref(),
+            &self.alt_aliases,
+            &self.room_id,
+        ))
     }
 
     fn unread_mentions(&self) -> u64 {
diff --git a/src/sliding_sync.rs b/src/sliding_sync.rs
@@ -48,7 +48,7 @@ use crate::{
         jump_to_bottom_button::UnreadMessageCount,
         popup_list::{enqueue_popup_notification, PopupItem, PopupKind}
     },
-    utils::{self, avatar_from_room_name, AVATAR_THUMBNAIL_FORMAT},
+    utils::{self, avatar_from_room_name, preferred_room_name, AVATAR_THUMBNAIL_FORMAT},
     verification::add_verification_event_handlers_and_sync_client
 };
 
@@ -2167,9 +2167,15 @@ async fn update_room(
         }
         if old_room.display_name != new_room.display_name {
             log!("Updating room {} name: {:?} --> {:?}", new_room_id, old_room.display_name, new_room.display_name);
+
+            let new_name = new_room
+                .display_name
+                .clone()
+                .unwrap_or(RoomDisplayName::Empty);
+
             enqueue_rooms_list_update(RoomsListUpdate::UpdateRoomName {
                 room_id: new_room_id.clone(),
-                new_room_name: new_room.display_name.as_ref().map(|n| n.to_string()),
+                new_room_name: new_name,
             });
         }
 
@@ -2328,8 +2334,12 @@ async fn add_new_room(
             let latest = latest_event.as_ref().map(
                 |ev| get_latest_event_details(ev, &new_room.room_id)
             );
-            let room_name = new_room.display_name.as_ref().map(|n| n.to_string());
-            let room_avatar = room_avatar(&new_room.room, room_name.as_deref()).await;
+            let room_display_name = new_room
+                .display_name
+                .clone()
+                .unwrap_or(RoomDisplayName::Empty);
+            let room_name_text = preferred_room_name(&room_display_name);
+            let room_avatar = room_avatar(&new_room.room, room_name_text.as_deref()).await;
 
             let inviter_info = if let Some(inviter) = invite_details.and_then(|d| d.inviter) {
                 Some(InviterInfo {
@@ -2347,7 +2357,7 @@ async fn add_new_room(
             };
             rooms_list::enqueue_rooms_list_update(RoomsListUpdate::AddInvitedRoom(InvitedRoomInfo {
                 room_id: new_room.room_id.clone(),
-                room_name,
+                room_name: room_display_name,
                 inviter_info,
                 room_avatar,
                 canonical_alias: new_room.room.canonical_alias(),
@@ -2406,7 +2416,11 @@ async fn add_new_room(
     // We need to add the room to the `ALL_JOINED_ROOMS` list before we can
     // send the `AddJoinedRoom` update to the UI, because the UI might immediately
     // issue a `MatrixRequest` that relies on that room being in `ALL_JOINED_ROOMS`.
-    let room_name = new_room.display_name.as_ref().map(|n| n.to_string());
+    let room_display_name = new_room
+        .display_name
+        .clone()
+        .unwrap_or(RoomDisplayName::Empty);
+    let room_name = preferred_room_name(&room_display_name);
     rooms_list::enqueue_rooms_list_update(RoomsListUpdate::AddJoinedRoom(JoinedRoomInfo {
         room_id: new_room.room_id.clone(),
         latest,
@@ -2415,7 +2429,7 @@ async fn add_new_room(
         num_unread_mentions: new_room.num_unread_mentions,
         // start with a basic text avatar; the avatar image will be fetched asynchronously below.
         avatar: avatar_from_room_name(room_name.as_deref()),
-        room_name,
+        room_name: room_display_name,
         canonical_alias: new_room.room.canonical_alias(),
         alt_aliases: new_room.room.alt_aliases(),
         has_been_paginated: false,
diff --git a/src/utils.rs b/src/utils.rs
