From f4db6988bf2fd71f74ac851557d82c6f65cc89b1 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 8 Nov 2020 18:57:44 +0100 Subject: More robust member profile data retrieval MemberEventContent: displayname and avatarUrl are now Omittables; CS API doesn't guarantee their presence (see also https://github.com/matrix-org/matrix-doc/issues/1375) but Quotient used to assume they are always there, causing #412. RoomMemberEvent: displayname() -> newDisplayName() and avatarUrl() -> newAvatarUrl(), to emphasise the actual semantics (and also the changed interface). The old signatures still work but are deprecated. Instead of roomMembername() (with weird camel-casing), three new methods in addition to safeMemberName() are introduced to Room: - memberName() - produces the "best known" display name for a given member; User::name() uses it to avoid the pitfall of #412. - disambiguatedMemberName() - this is what roomMembername() used to be; not recommended for direct use when UI is concerned. - safeMemberName() - remains as is, with the fix to the documentation that used to mislead that the function returns HTML-escaped content (it didn't, and doesn't). - htmlSafeMemberName() - does what safeMemberName() claimed to do. Respectively, memberNames() is deprecated in favor of safeMemberNames() and htmlSafeMemberNames(). The corresponding Q_PROPERTY uses safeMemberNames() now. Similar to memberName(), Room has got memberAvatarUrl() to spare User class from diving into Room state to find the member avatar URL. Closes #412. --- lib/room.cpp | 85 +++++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 67 insertions(+), 18 deletions(-) (limited to 'lib/room.cpp') diff --git a/lib/room.cpp b/lib/room.cpp index f8e6e6ba..b564369c 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1241,12 +1241,27 @@ QList Room::membersLeft() const { return d->membersLeft; } QList Room::users() const { return d->membersMap.values(); } -QStringList Room::memberNames() const +[[deprecated]] QStringList Room::memberNames() const +{ + return safeMemberNames(); +} + +QStringList Room::safeMemberNames() const +{ + QStringList res; + res.reserve(d->membersMap.size()); + for (auto u: std::as_const(d->membersMap)) + res.append(safeMemberName(u->id())); + + return res; +} + +QStringList Room::htmlSafeMemberNames() const { QStringList res; res.reserve(d->membersMap.size()); - for (auto u : qAsConst(d->membersMap)) - res.append(roomMembername(u)); + for (auto u: std::as_const(d->membersMap)) + res.append(htmlSafeMemberName(u->id())); return res; } @@ -1411,37 +1426,71 @@ Room::Private::moveEventsToTimeline(RoomEventsRange events, return insertedSize; } +QString Room::memberName(const QString& mxId) const +{ + // See https://github.com/matrix-org/matrix-doc/issues/1375 + const auto rme = getCurrentState(mxId); + return rme->newDisplayName() ? *rme->newDisplayName() + : rme->prevContent() ? rme->prevContent()->displayName.value_or(QString()) + : QString(); +} + QString Room::roomMembername(const User* u) const +{ + Q_ASSERT(u != nullptr); + return disambiguatedMemberName(u->id()); +} + +QString Room::roomMembername(const QString& userId) const +{ + return disambiguatedMemberName(userId); +} + +inline QString makeFullUserName(const QString& displayName, const QString& mxId) +{ + return displayName % " (" % mxId % ')'; +} + +QString Room::disambiguatedMemberName(const QString& mxId) const { // See the CS spec, section 11.2.2.3 - const auto username = u->name(this); + const auto username = memberName(mxId); if (username.isEmpty()) - return u->id(); + return mxId; auto namesakesIt = qAsConst(d->membersMap).find(username); // We expect a user to be a member of the room - but technically it is - // possible to invoke roomMemberName() even for non-members. In such case + // possible to invoke this function even for non-members. In such case // we return the full name, just in case. if (namesakesIt == d->membersMap.cend()) - return u->fullName(this); + return makeFullUserName(username, mxId); auto nextUserIt = namesakesIt; if (++nextUserIt == d->membersMap.cend() || nextUserIt.key() != username) return username; // No disambiguation necessary - return u->fullName(this); // Disambiguate fully + return makeFullUserName(username, mxId); // Disambiguate fully } -QString Room::roomMembername(const QString& userId) const +QString Room::safeMemberName(const QString& userId) const { - return roomMembername(user(userId)); + return sanitized(roomMembername(userId)); } -QString Room::safeMemberName(const QString& userId) const +QString Room::htmlSafeMemberName(const QString& userId) const { - return sanitized(roomMembername(userId)); + return safeMemberName(userId).toHtmlEscaped(); +} + +QUrl Room::memberAvatarUrl(const QString &mxId) const +{ + // See https://github.com/matrix-org/matrix-doc/issues/1375 + const auto rme = getCurrentState(mxId); + return rme->newAvatarUrl() ? *rme->newAvatarUrl() + : rme->prevContent() ? rme->prevContent()->avatarUrl.value_or(QUrl()) + : QUrl(); } void Room::updateData(SyncRoomData&& data, bool fromCache) @@ -2422,8 +2471,8 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) case MembershipType::Join: switch (rme.membership()) { case MembershipType::Join: // rename/avatar change or no-op - if (rme.displayName() != oldRme->displayName()) { - emit memberAboutToRename(u, rme.displayName()); + if (rme.newDisplayName()) { + emit memberAboutToRename(u, *rme.newDisplayName()); d->removeMemberFromMap(u->name(this), u); } break; @@ -2517,11 +2566,11 @@ Room::Changes Room::processStateEvent(const RoomEvent& e) d->insertMemberIntoMap(u); emit userAdded(u); } else { - if (oldMemberEvent->displayName() != evt.displayName()) { + if (evt.newDisplayName()) { d->insertMemberIntoMap(u); emit memberRenamed(u); } - if (oldMemberEvent->avatarUrl() != evt.avatarUrl()) + if (evt.newAvatarUrl()) emit memberAvatarChanged(u); } break; @@ -2858,12 +2907,12 @@ MemberSorter Room::memberSorter() const { return MemberSorter(this); } bool MemberSorter::operator()(User* u1, User* u2) const { - return operator()(u1, room->roomMembername(u2)); + return operator()(u1, room->disambiguatedMemberName(u2->id())); } bool MemberSorter::operator()(User* u1, const QString& u2name) const { - auto n1 = room->roomMembername(u1); + auto n1 = room->disambiguatedMemberName(u1->id()); if (n1.startsWith('@')) n1.remove(0, 1); auto n2 = u2name.midRef(u2name.startsWith('@') ? 1 : 0); -- cgit v1.2.3