From bd9cb5aae5c9311f071d8fc932a6e5478bf79bdb Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 09:51:00 +0200 Subject: [PATCH 01/11] add RoomMember.name which falls back to userId this will prevent the crash when left members have their displayname removed (another issue) --- src/matrix/room/members/Heroes.js | 8 ++++---- src/matrix/room/members/RoomMember.js | 13 +++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/matrix/room/members/Heroes.js b/src/matrix/room/members/Heroes.js index 79902579..74f69db1 100644 --- a/src/matrix/room/members/Heroes.js +++ b/src/matrix/room/members/Heroes.js @@ -22,12 +22,12 @@ function calculateRoomName(sortedMembers, summary) { if (sortedMembers.length > 1) { const lastMember = sortedMembers[sortedMembers.length - 1]; const firstMembers = sortedMembers.slice(0, sortedMembers.length - 1); - return firstMembers.map(m => m.displayName).join(", ") + " and " + lastMember.displayName; + return firstMembers.map(m => m.name).join(", ") + " and " + lastMember.name; } else { - return sortedMembers[0].displayName; + return sortedMembers[0].name; } } else if (sortedMembers.length < countWithoutMe) { - return sortedMembers.map(m => m.displayName).join(", ") + ` and ${countWithoutMe} others`; + return sortedMembers.map(m => m.name).join(", ") + ` and ${countWithoutMe} others`; } else { // Empty Room return null; @@ -81,7 +81,7 @@ export class Heroes { for (const member of updatedHeroMembers) { this._members.set(member.userId, member); } - const sortedMembers = Array.from(this._members.values()).sort((a, b) => a.displayName.localeCompare(b.displayName)); + const sortedMembers = Array.from(this._members.values()).sort((a, b) => a.name.localeCompare(b.name)); this._roomName = calculateRoomName(sortedMembers, summary); } diff --git a/src/matrix/room/members/RoomMember.js b/src/matrix/room/members/RoomMember.js index e6303fe4..6157c42e 100644 --- a/src/matrix/room/members/RoomMember.js +++ b/src/matrix/room/members/RoomMember.js @@ -54,10 +54,23 @@ export class RoomMember { }); } + /** + * @return {String?} the display name, if any + */ get displayName() { return this._data.displayName; } + /** + * @return {String} the display name or userId + */ + get name() { + return this._data.displayName || this._data.userId; + } + + /** + * @return {String?} the avatar mxc url, if any + */ get avatarUrl() { return this._data.avatarUrl; } From 34ec96c1b86caa67334efacbfc23b49851e038c4 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 09:51:44 +0200 Subject: [PATCH 02/11] look for displayname/avatar in prev content as well as synapse doesn't set them on content for leave memberships this caused these props to be removed in storage --- src/matrix/room/members/RoomMember.js | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/matrix/room/members/RoomMember.js b/src/matrix/room/members/RoomMember.js index 6157c42e..02c3c292 100644 --- a/src/matrix/room/members/RoomMember.js +++ b/src/matrix/room/members/RoomMember.js @@ -27,21 +27,33 @@ export class RoomMember { if (typeof userId !== "string") { return; } - return this._fromMemberEventContent(roomId, userId, memberEvent.content); + const content = memberEvent.content; + const prevContent = memberEvent.unsigned?.prev_content; + const membership = content?.membership; + // fall back to prev_content for these as synapse doesn't (always?) + // put them on content for "leave" memberships + const displayName = content?.displayname || prevContent?.displayname; + const avatarUrl = content?.avatar_url || prevContent?.avatar_url; + return this._validateAndCreateMember(roomId, userId, membership, displayName, avatarUrl); } - + /** + * Creates a (historical) member from a member event that is the next member event + * after the point in time where we need a member for. This will use `prev_content`. + */ static fromReplacingMemberEvent(roomId, memberEvent) { const userId = memberEvent && memberEvent.state_key; if (typeof userId !== "string") { return; } - return this._fromMemberEventContent(roomId, userId, memberEvent.prev_content); + const content = memberEvent.unsigned?.prev_content + return this._validateAndCreateMember(roomId, userId, + content?.membership, + content?.displayname, + content?.avatar_url + ); } - static _fromMemberEventContent(roomId, userId, content) { - const membership = content?.membership; - const avatarUrl = content?.avatar_url; - const displayName = content?.displayname; + static _validateAndCreateMember(roomId, userId, membership, displayName, avatarUrl) { if (typeof membership !== "string") { return; } From 1fe496eeea4670fb901eae604f8cca9c4160784f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 09:52:30 +0200 Subject: [PATCH 03/11] fix crash when state is not set (erroneously?) on gap response this seems to happen when the only event in the room is a m.room.create --- src/matrix/room/timeline/persistence/GapWriter.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/matrix/room/timeline/persistence/GapWriter.js b/src/matrix/room/timeline/persistence/GapWriter.js index 834239f7..e9abded6 100644 --- a/src/matrix/room/timeline/persistence/GapWriter.js +++ b/src/matrix/room/timeline/persistence/GapWriter.js @@ -142,8 +142,9 @@ export class GapWriter { return RoomMember.fromReplacingMemberEvent(this._roomId, event)?.serialize(); } } - // assuming the member hasn't changed within the chunk, just take it from state if it's there - const stateMemberEvent = state.find(isOurUser); + // assuming the member hasn't changed within the chunk, just take it from state if it's there. + // Don't assume state is set though, as it can be empty at the top of the timeline in some circumstances + const stateMemberEvent = state?.find(isOurUser); if (stateMemberEvent) { return RoomMember.fromMemberEvent(this._roomId, stateMemberEvent)?.serialize(); } From 59443e6602cbefdabdf601dd48d5eec1fd0709db Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 10:07:47 +0200 Subject: [PATCH 04/11] close the room tile view model as well when closing a room so it does not stay selected --- src/domain/session/SessionViewModel.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/domain/session/SessionViewModel.js b/src/domain/session/SessionViewModel.js index 879c5e26..152b77ac 100644 --- a/src/domain/session/SessionViewModel.js +++ b/src/domain/session/SessionViewModel.js @@ -57,24 +57,20 @@ export class SessionViewModel extends ViewModel { } _closeCurrentRoom() { - if (this._currentRoomViewModel) { - this._currentRoomViewModel = this.disposeTracked(this._currentRoomViewModel); - this.emitChange("currentRoom"); - } + this._currentRoomTileViewModel?.close(); + this._currentRoomViewModel = this.disposeTracked(this._currentRoomViewModel); } _openRoom(room, roomTileVM) { - if (this._currentRoomTileViewModel) { - this._currentRoomTileViewModel.close(); - } + this._closeCurrentRoom(); this._currentRoomTileViewModel = roomTileVM; - if (this._currentRoomViewModel) { - this._currentRoomViewModel = this.disposeTracked(this._currentRoomViewModel); - } this._currentRoomViewModel = this.track(new RoomViewModel(this.childOptions({ room, ownUserId: this._session.user.id, - closeCallback: () => this._closeCurrentRoom(), + closeCallback: () => { + this._closeCurrentRoom(); + this.emitChange("currentRoom"); + }, }))); this._currentRoomViewModel.load(); this.emitChange("currentRoom"); From 3e8e1bab67887e8d2198af0cc9cdf9709e1a1601 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 10:38:22 +0200 Subject: [PATCH 05/11] remove logging --- src/matrix/room/members/Heroes.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/matrix/room/members/Heroes.js b/src/matrix/room/members/Heroes.js index 74f69db1..f7ccb2df 100644 --- a/src/matrix/room/members/Heroes.js +++ b/src/matrix/room/members/Heroes.js @@ -92,7 +92,6 @@ export class Heroes { get roomAvatarUrl() { if (this._members.size === 1) { for (const member of this._members.values()) { - console.log("roomAvatarUrl", member, member.avatarUrl); return member.avatarUrl; } } From 41a7448c7419b0102bac4740b58fa91a5ea52a2f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 10:40:49 +0200 Subject: [PATCH 06/11] add logging for room list sorting --- .../session/roomlist/RoomTileViewModel.js | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/src/domain/session/roomlist/RoomTileViewModel.js b/src/domain/session/roomlist/RoomTileViewModel.js index ec344f79..25ebf30e 100644 --- a/src/domain/session/roomlist/RoomTileViewModel.js +++ b/src/domain/session/roomlist/RoomTileViewModel.js @@ -49,34 +49,59 @@ export class RoomTileViewModel extends ViewModel { } compare(other) { + /* + put unread rooms first + then put rooms with a timestamp first, and sort by name + then sort by name for rooms without a timestamp + */ const myRoom = this._room; const theirRoom = other._room; + let buf = ""; + function log(...args) { + buf = buf + args.join(" ") + "\n"; + } + function logResult(result) { + if (result === 0) { + log("rooms are equal (should not happen)", result); + } else if (result > 0) { + log(`${theirRoom.name || theirRoom.id} comes first`, result); + } else { + log(`${myRoom.name || myRoom.id} comes first`, result); + } + console.log(buf); + return result; + } + log(`comparing ${myRoom.name || theirRoom.id} and ${theirRoom.name || theirRoom.id} ...`); + log("comparing isUnread..."); if (isSortedAsUnread(this) !== isSortedAsUnread(other)) { if (isSortedAsUnread(this)) { - return -1; + return logResult(-1); } - return 1; + return logResult(1); } const myTimestamp = myRoom.lastMessageTimestamp; const theirTimestamp = theirRoom.lastMessageTimestamp; // rooms with a timestamp come before rooms without one if ((myTimestamp === null) !== (theirTimestamp === null)) { + log("checking if either does not have lastMessageTimestamp ..."); if (theirTimestamp === null) { - return -1; + return logResult(-1); } - return 1; + return logResult(1); } const timeDiff = theirTimestamp - myTimestamp; - if (timeDiff === 0) { + if (timeDiff === 0 || !Number.isSafeInteger(theirTimestamp) || !Number.isSafeInteger(myTimestamp)) { + log("checking name ..."); // sort alphabetically const nameCmp = this.name.localeCompare(other.name); if (nameCmp === 0) { - return this._room.id.localeCompare(other._room.id); + return logResult(this._room.id.localeCompare(other._room.id)); } - return nameCmp; + return logResult(nameCmp); } - return timeDiff; + log("checking timestamp ..."); + return logResult(timeDiff); } get isOpen() { From 4b682ad9305bea86df5356abce7f86d659ed1d2d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 10:45:20 +0200 Subject: [PATCH 07/11] use the same check when seeing if either does not have a timestamp --- src/domain/session/roomlist/RoomTileViewModel.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/domain/session/roomlist/RoomTileViewModel.js b/src/domain/session/roomlist/RoomTileViewModel.js index 25ebf30e..5a73a86a 100644 --- a/src/domain/session/roomlist/RoomTileViewModel.js +++ b/src/domain/session/roomlist/RoomTileViewModel.js @@ -82,8 +82,10 @@ export class RoomTileViewModel extends ViewModel { } const myTimestamp = myRoom.lastMessageTimestamp; const theirTimestamp = theirRoom.lastMessageTimestamp; - // rooms with a timestamp come before rooms without one - if ((myTimestamp === null) !== (theirTimestamp === null)) { + const myTimestampValid = Number.isSafeInteger(myTimestamp); + const theirTimestampValid = Number.isSafeInteger(theirTimestamp); + // if either does not have a timestamp, put the one with a timestamp first + if (myTimestampValid !== theirTimestampValid) { log("checking if either does not have lastMessageTimestamp ..."); if (theirTimestamp === null) { return logResult(-1); @@ -91,7 +93,7 @@ export class RoomTileViewModel extends ViewModel { return logResult(1); } const timeDiff = theirTimestamp - myTimestamp; - if (timeDiff === 0 || !Number.isSafeInteger(theirTimestamp) || !Number.isSafeInteger(myTimestamp)) { + if (timeDiff === 0 || !theirTimestampValid || !myTimestampValid) { log("checking name ..."); // sort alphabetically const nameCmp = this.name.localeCompare(other.name); From 9e891c34425ae777436439ef1d7bebc7823916ac Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 10:48:12 +0200 Subject: [PATCH 08/11] log actual timestamps as well so we can see if they are anything but null or a number --- src/domain/session/roomlist/RoomTileViewModel.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/domain/session/roomlist/RoomTileViewModel.js b/src/domain/session/roomlist/RoomTileViewModel.js index 5a73a86a..00c6df9d 100644 --- a/src/domain/session/roomlist/RoomTileViewModel.js +++ b/src/domain/session/roomlist/RoomTileViewModel.js @@ -59,7 +59,7 @@ export class RoomTileViewModel extends ViewModel { let buf = ""; function log(...args) { - buf = buf + args.join(" ") + "\n"; + buf = buf + args.map(a => a+"").join(" ") + "\n"; } function logResult(result) { if (result === 0) { @@ -86,7 +86,7 @@ export class RoomTileViewModel extends ViewModel { const theirTimestampValid = Number.isSafeInteger(theirTimestamp); // if either does not have a timestamp, put the one with a timestamp first if (myTimestampValid !== theirTimestampValid) { - log("checking if either does not have lastMessageTimestamp ..."); + log("checking if either does not have lastMessageTimestamp ...", myTimestamp, theirTimestamp); if (theirTimestamp === null) { return logResult(-1); } @@ -94,7 +94,7 @@ export class RoomTileViewModel extends ViewModel { } const timeDiff = theirTimestamp - myTimestamp; if (timeDiff === 0 || !theirTimestampValid || !myTimestampValid) { - log("checking name ..."); + log("checking name ...", myTimestamp, theirTimestamp); // sort alphabetically const nameCmp = this.name.localeCompare(other.name); if (nameCmp === 0) { From 1a6931129b1cedd5c490901082e5ace9a8cb2f7c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 10:50:30 +0200 Subject: [PATCH 09/11] log as info --- src/domain/session/roomlist/RoomTileViewModel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/domain/session/roomlist/RoomTileViewModel.js b/src/domain/session/roomlist/RoomTileViewModel.js index 00c6df9d..bed129e2 100644 --- a/src/domain/session/roomlist/RoomTileViewModel.js +++ b/src/domain/session/roomlist/RoomTileViewModel.js @@ -69,7 +69,7 @@ export class RoomTileViewModel extends ViewModel { } else { log(`${myRoom.name || myRoom.id} comes first`, result); } - console.log(buf); + console.info(buf); return result; } log(`comparing ${myRoom.name || theirRoom.id} and ${theirRoom.name || theirRoom.id} ...`); From 8543ec00d05d54bba58062d9d58b94e3f48ec95f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 10:52:38 +0200 Subject: [PATCH 10/11] release v0.0.33 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 9786ce4b..681ee055 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "hydrogen-web", - "version": "0.0.32", + "version": "0.0.33", "description": "A javascript matrix client prototype, trying to minize RAM usage by offloading as much as possible to IndexedDB", "main": "index.js", "directories": { From 05821b0fdfad8e536294175bb551ef407508769f Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 12:42:38 +0200 Subject: [PATCH 11/11] use same check for timestamp validity as timestamp can be undefined sometimes --- src/domain/session/roomlist/RoomTileViewModel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/domain/session/roomlist/RoomTileViewModel.js b/src/domain/session/roomlist/RoomTileViewModel.js index bed129e2..e586bbd1 100644 --- a/src/domain/session/roomlist/RoomTileViewModel.js +++ b/src/domain/session/roomlist/RoomTileViewModel.js @@ -87,7 +87,7 @@ export class RoomTileViewModel extends ViewModel { // if either does not have a timestamp, put the one with a timestamp first if (myTimestampValid !== theirTimestampValid) { log("checking if either does not have lastMessageTimestamp ...", myTimestamp, theirTimestamp); - if (theirTimestamp === null) { + if (!theirTimestampValid) { return logResult(-1); } return logResult(1);