Use `RoomDisplayName` everywhere to ensure that room names display correctly

[TigerInYourDream]

Bug Fix: Invited Room Names Not Displaying Correctly

Problem Description

When receiving room invitations in Robrix, the room names displayed as "Invite to unnamed room" instead of showing the actual room name (e.g., "room0", "room1").

Root Cause Analysis

1. Initial Problem: UpdateRoomName Not Handling Invited Rooms

The RoomsListUpdate::UpdateRoomName handler only searched in all_joined_rooms, but invited rooms are stored in a separate collection invited_rooms.

Location: src/home/rooms_list.rs:458-492

RoomsListUpdate::UpdateRoomName { room_id, new_room_name } => {
    if let Some(room) = self.all_joined_rooms.get_mut(&room_id) {
        // Updates joined room name
    }
    // ❌ Missing: check invited_rooms
}

2. Matrix Protocol Background: "Empty Room" Placeholder

According to the Matrix Client-Server API Specification:

Calculating Room Display Names:

If the room has an m.room.name state event, use the name given by that event.

If the room has an m.room.canonical_alias state event, use the alias given by that event.

If neither of the above events are present, a name should be composed based on the members of the room.

A complete absence of m.room.name, m.room.canonical_alias, and m.room.member events is likely to indicate a problem with creating the room or synchronising the state table; however clients should still handle this situation. A display name such as "Empty Room" (or a localised variant thereof) should be used in this situation.

The Matrix Rust SDK implements this specification in Room::display_name() and returns "Empty Room" as a placeholder when no room information is available yet.

3. Timing Issue with Sliding Sync

For invited rooms, there's a timing sequence:

1. Initial sync: Robrix receives basic invitation info via sliding sync

   • At this point, room state events haven't been synchronized yet
   • room.display_name() returns Some("Empty Room") (Matrix SDK's placeholder)

2. Subscription: We call room_list_service.subscribe_to_rooms(&[&room_id])

   • The SDK starts fetching complete room state

3. State events arrive: Room name, alias, and member information arrive

   • room.display_name() updates to return the actual room name
   • update_room() is called with the new name

4. SDK Internal Architecture Issue

The RoomListServiceRoomInfo struct stores a reference to the SDK's Room object:

struct RoomListServiceRoomInfo {
    room: matrix_sdk::Room,  // ← Reference to SDK's internal Arc<RoomInfo>
}

When update_room() is called:

let old_cached_name = old_room.room.cached_display_name();  // ← Already updated!
let new_cached_name = new_room.cached_display_name();       // ← Same value!

Both old_room.room and new_room reference the same internal object in the Matrix SDK (via Arc), so cached_display_name() always returns the latest value, making it impossible to detect changes using this approach.

Solution

Change 1: Handle Invited Rooms in UpdateRoomName Handler

File: src/home/rooms_list.rs

RoomsListUpdate::UpdateRoomName { room_id, new_room_name } => {
    if let Some(room) = self.all_joined_rooms.get_mut(&room_id) {
        room.room_name = Some(new_room_name);
        self.redraw(cx);
    } else if let Some(invited_room) = self.invited_rooms.borrow_mut().get_mut(&room_id) {
        // ✅ Added: handle invited rooms
        invited_room.room_name = Some(new_room_name);
    } else {
        error!("Error: couldn't find room {room_id} to update room name");
    }
}

Change 2: Subscribe to Invited Rooms for State Updates

File: src/sliding_sync.rs:2116

RoomState::Invited => {
    // ... existing code ...

    // ✅ Added: Subscribe to receive state updates (like room name changes)
    room_list_service.subscribe_to_rooms(&[&room_id]).await;

    // ... send AddInvitedRoom update ...
}

Change 3: Handle "Empty Room" Placeholder

File: src/sliding_sync.rs:2079-2085

RoomState::Invited => {
    // For invited rooms with "Empty Room" name, set it to None
    // so UI shows "Invite to unnamed room"
    // The name will be updated later when we receive room state events
    let room_name = if room_name.as_deref() == Some("Empty Room") {
        None  // ✅ Convert SDK placeholder to None for better UX
    } else {
        room_name
    };

    // ... rest of invitation handling ...
}

Rationale: "Empty Room" is a placeholder indicating missing data, not an actual room name. Displaying "Invite to unnamed room" provides better user experience while waiting for the real name to arrive.

Change 4: Special Handling for Invited Room Name Updates

File: src/sliding_sync.rs:1984-2003

if let Some(new_room_name) = new_room_name {
    let old_cached_name = old_room.room.cached_display_name().map(|n| n.to_string());

    // For invited rooms, always send update if the name is not "Empty Room"
    // because we might have initially set the name to None in add_new_room,
    // but the Matrix SDK's cached_display_name() already reflects the updated name
    // (since old_room.room is a reference to SDK's internal object, not a snapshot).
    let should_update = if new_room_state == RoomState::Invited {
        new_room_name != "Empty Room"  // ✅ Skip placeholder, send real names
    } else {
        old_cached_name.as_ref() != Some(&new_room_name)
    };

    if should_update {
        enqueue_rooms_list_update(RoomsListUpdate::UpdateRoomName {
            room_id: new_room_id.clone(),
            new_room_name,
        });
    }
}

Trade-off: This may send duplicate updates (e.g., "room4" → "room4") during subsequent syncs. However, this is acceptable because:

• The overhead is minimal (a few channel messages, ~100 bytes each)
• It ensures the initial update is never skipped
• UI frameworks typically handle redundant updates efficiently

Testing

Test Case 1: Fresh Invitation

1. Start Robrix with clean database
2. Send invitation for "testroom" from another client
3. Expected: Robrix UI displays "testroom" (not "Invite to unnamed room")

Test Case 2: Restored Invitations

1. Restart Robrix with existing invitations in database
2. Expected: All invited rooms display correct names immediately

Test Results

✅ Tested with room0 through room9 ✅ All room names display correctly ✅ No "Invite to unnamed room" for named rooms

Impact

Files Modified

• src/home/rooms_list.rs (2 lines added)
• src/sliding_sync.rs (multiple changes)

Behavior Changes

• Before: Invited rooms always showed "Invite to unnamed room"
• After: Invited rooms show actual room names within ~100ms of receiving invitation

Performance Impact

• Minimal: Additional subscription per invited room
• Small overhead: Possible duplicate name updates (acceptable trade-off)

Related Issues

Matrix SDK Behavior

• The SDK's Room::display_name() returns "Empty Room" as per Matrix specification
• The SDK uses Arc internally, making Room a reference to shared state
• This means old_room.room.cached_display_name() always returns the latest value

Alternative Approaches Considered

1. Track sent names in a separate Map

   • ❌ Rejected: Added complexity, potential memory leaks
   • ❌ Need to handle room removal edge cases

2. Store sent name in RoomListServiceRoomInfo

   • ❌ Rejected: Requires snapshot of room state, not just reference

3. Current approach: Accept duplicate updates

   • ✅ Chosen: Simple, correct, minimal overhead

References

• Matrix Client-Server API: Calculating Display Names
• Matrix Rust SDK: Room::display_name() implementation
• Matrix Rust SDK: Sliding Sync and Room List Service

---

Date: 2025-09-30 PR Branch: fix/invited-room-name-display

[kevinaboos]

Is this ready? If so (and in the future), please re-request a review from me and add the waiting-on-review label.

[kevinaboos]

@TigerInYourDream I just happened to make some changes to the RoomsListServiceRoomInfo struct in #611 that are similar to what you need in this PR. Take a look, and ensure the merge conflicts are carefully resolved.

[TigerInYourDream]

After going through a few files, I'm curious about the usage of RoomDisplayName everywhere instead of RoomName. I didn't expect to see that; I thought the goal was to use the new RoomName everywhere for ease of use?

Also, after seeing the new code, we should probably just make RoomName contain both a RoomDisplayName and and OwnedRoomId. That way you can impl Display for RoomName, in which the RoomDisplayName::Empty variant automatically uses the OwnedRoomId within the RoomName struct itself. You can then remove the utils::room_name_or_id() function, which is tedious to use.

Thanks for taking another look. The reason the last PR still uses RoomDisplayName in most places is that RoomName is currently just a thin newtype over it (see utils::RoomName). Outside of the helpers that need fallback logic (room_name_or_id, avatar generation, etc.), switching every struct to the wrapper would only add a layer of wrap/unwrap without giving us additional semantics—especially because many paths still need to interact with the SDK’s RoomDisplayName variants directly (EmptyWas, Calculated, …) and to serialize/deserialize them as-is. I only used RoomName where the fallback behavior was actually needed, and left the rest untouched to keep the churn down and preserve the SDK-facing types.

That said, I agree this leaves us with two types representing the same concept, which hurts consistency. I like your suggestion of evolving RoomName into a richer type that also carries an OwnedRoomId (or introducing an equivalent “known room” struct), so we can implement Display once and drop room_name_or_id(). I’ll plan a follow-up patch to converge on that single type and update the call sites that currently only know the display name to either provide the ID or opt into a different helper.

[kevinaboos]

Please apply the suggested changes in this PR, in particular the RoomName type redesign. I've already spent a long time reviewing this PR in detail, and I don't think it's beneficial to save that for another PR, because it would end up touching all the same lines of code as this one.

[TigerInYourDream]

• I introduced a canonical RoomName { display_name: RoomDisplayName, room_id: OwnedRoomId } so the UI/services/logging layers don’t have to juggle parallel IDs and fallback strings. With this in place, I removed redundant room_id fields in structs that already carry a RoomName (e.g. SelectedRoom, InviteScreen, RoomScreen) and updated the relevant functions to take a single RoomName instead of loosely-coupled (room_id, room_name) arguments.
• I did a repo-wide sweep to ensure every call site that needs both the friendly display text and the real room ID relies on RoomName. This touches a lot of files, but the aim is consistency—no more Option<String> names or helper functions like room_name_or_id(). To keep the ergonomics reasonable, I added helpers such as display_str() / room_id_str() and an IntoRoomName trait, plus ~40 lines of unit tests covering the fallback behavior and the new SerRon/DeRon implementation.
• The tricky part is serialization. The old SelectedRoom persisted:

		  #[derive(Clone, Debug, SerRon, DeRon)]
		  pub enum SelectedRoom {
		      JoinedRoom {
		          room_id: OwnedRoomIdRon,
		          room_name: Option<String>,
		      },
		      InvitedRoom {
		          room_id: OwnedRoomIdRon,
		          room_name: Option<String>,
		      },
		  }

where RoomName was effectively just a string wrapper encoded as JSON-in-RON. Now SelectedRoom holds a single RoomName, serialized as a native (RoomDisplayNameRon, OwnedRoomIdRon) tuple. That means existing latest_app_state.ron files can’t be read by the new code. I’m planning to fix this by adding fallback logic in load_app_state: first attempt to deserialize the new format; if that fails, parse the legacy structure and convert it into the new RoomName before returning. Because saving still writes the new format, users automatically migrate once the state is rewritten.

Questions for reviewers:

1. For serialization compatibility, is the fallback-and-convert approach acceptable, or would you rather keep the original { room_id, room_name } on disk even though it undermines the “RoomName carries both pieces” consolidation?
2. Is the ~40-line unit test module worth keeping for the extra coverage, or should I drop it to keep the PR smaller?

[TigerInYourDream]

This UPDATE can be digested in three buckets:

1. Canonical RoomName type
   • Introduced RoomName { display_name: RoomDisplayName, room_id: OwnedRoomId } so every UI/service/logging layer can pass a single value instead of juggling room_id plus various optional name fields.
   • SelectedRoom, InviteScreen, RoomScreen, and other structs that previously duplicated both fields now store just RoomName, and their APIs accept RoomName directly.
   • Added helpers like display_str(), room_id_str(), and IntoRoomName, plus ~40 lines of unit tests covering the fallback behavior and the updated SerRon/DeRon implementation.
2. Repo-wide call-site sweep
   • Every call site that needs “friendly text + real room ID” now relies on RoomName.
   • Removed legacy Option<String> fields and helpers such as room_name_or_id() so UI rendering, logging, and filtering/sorting all use the same entry point.
   • As a result, RoomName automatically handles Empty / EmptyWas fallback semantics everywhere.
3. Persistence changes & compatibility stance
   • SelectedRoom now serializes a single RoomName as a (RoomDisplayNameRon, OwnedRoomIdRon) tuple instead of encoding it as JSON-in-RON.
   • Legacy latest_app_state.ron files don’t match the new schema, so load_app_state now tries the new format first and, on failure, logs an error, backs up the old file, and returns the default app state. Because we’re still pre-alpha and reviewers confirmed breaking changes are acceptable, we opted for this fail-safe instead of migrating the old structure.
   • In the next PR I plan to migrate the entire app-state persistence layer to Serde + JSON, which will let us drop RON/micro-serde and the transitional helper types entirely.

Hope this helps reviewers grasp the scope quickly and sets expectations for the follow-up work.

[kevinaboos]

Questions for reviewers:

1. For serialization compatibility, is the fallback-and-convert approach acceptable, or would you rather keep the original { room_id, room_name } on disk even though it undermines the “RoomName carries both pieces” consolidation?
2. Is the ~40-line unit test module worth keeping for the extra coverage, or should I drop it to keep the PR smaller?

I believe I responded to this on our Matrix channel, but I'll echo my response here for posterity's sake:

1. Don't worry about backwards compatibility right now, we have no real userbase. You can make breaking changes. In this case I think we should probably stop using RON and just use JSON for ease of struct design.
2. of course you can keep testing code in the repo, as long as it's within a #[cfg(test)] attribute.

[kevinaboos]

This UPDATE can be digested in three buckets: ...

Thanks for the excellent summary. I agree with all of these design changes; I'm starting a final review now.

[TigerInYourDream]

Summary of Changes Since Last Review

Key Changes

1. Removed IntoRoomName trait - Using standard From trait with .into() instead
2. Removed display_str() method - Using Display trait with .to_string() everywhere
3. Simplified stringify_join_leave_error() - Now accepts &RoomNameId directly, no Option wrapper
4. Fixed rooms_list.rs redundancies: - Using self.invited_rooms instead of get_invited_rooms(cx) - Removed unnecessary .clone() calls - Removed redundant .contains() checks
5. Fixed restore_status_view logic - Always calling set_content() before draw()
6. Restored comments and whitespace - All important comments and empty lines restored
7. Restored log message info - Room name now included in "room loaded" log
8. Field rename - room_name → room_name_id for consistency
9. Restored dummy room props fallback - Kept original logic for handling events when no room is selected, adapted to use RoomNameId