From 372ed74c04f2c542451771aa792242a4e2afb351 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 20 Oct 2016 09:38:16 +0900 Subject: Room: added setLastReadEvent accessor and a signal for it; don't post receipts for own messages to the server As discussed with Matthew in #quaternion: https://matrix.to/#/!PCzUtxtOjUySxSelof:matrix.org/$14768896199130qcJqe:matrix.org --- room.cpp | 12 ++++++++++-- room.h | 2 ++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/room.cpp b/room.cpp index e07426a7..5181fb2a 100644 --- a/room.cpp +++ b/room.cpp @@ -163,9 +163,17 @@ void Room::setJoinState(JoinState state) emit joinStateChanged(oldState, state); } +void Room::setLastReadEvent(User* user, QString eventId) +{ + d->lastReadEvent.insert(user, eventId); + emit lastReadEventChanged(user); +} + void Room::markMessageAsRead(Event* event) { - d->connection->postReceipt(this, event); + setLastReadEvent(connection()->user(), event->id()); + if (event->senderId() != connection()->userId()) + d->connection->postReceipt(this, event); } QString Room::lastReadEvent(User* user) @@ -489,7 +497,7 @@ void Room::processEphemeralEvent(Event* event) for( const Receipt& r: receipts ) { if (auto m = d->member(r.userId)) - d->lastReadEvent.insert(m, eventId); + setLastReadEvent(m, eventId); } } } diff --git a/room.h b/room.h index 7266ae70..3727af0e 100644 --- a/room.h +++ b/room.h @@ -69,6 +69,7 @@ namespace QMatrixClient Q_INVOKABLE void updateData(SyncRoomData& data ); Q_INVOKABLE void setJoinState( JoinState state ); + Q_INVOKABLE void setLastReadEvent(User* user, QString eventId); Q_INVOKABLE void markMessageAsRead( Event* event ); Q_INVOKABLE QString lastReadEvent(User* user); @@ -102,6 +103,7 @@ namespace QMatrixClient void typingChanged(); void highlightCountChanged(Room* room); void notificationCountChanged(Room* room); + void lastReadEventChanged(User* user); protected: Connection* connection() const; -- cgit v1.2.3 From c8700b59d00d505c59de51d8dc259454073123e3 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 21 Oct 2016 12:51:01 +0900 Subject: Room::markMessagesAsRead correctly handles local user's messages now setLastReadEvent() is called in any case (read marks a stored in a hashmap so it's a constant time operation anyway); postReceipt() is now called for the nearest previous non-local message. --- room.cpp | 29 +++++++++++++++++++++++++---- room.h | 17 +++++++++++++++-- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/room.cpp b/room.cpp index 5181fb2a..674f0343 100644 --- a/room.cpp +++ b/room.cpp @@ -169,11 +169,32 @@ void Room::setLastReadEvent(User* user, QString eventId) emit lastReadEventChanged(user); } -void Room::markMessageAsRead(Event* event) +void Room::markMessagesAsRead(Timeline::const_iterator end) { - setLastReadEvent(connection()->user(), event->id()); - if (event->senderId() != connection()->userId()) - d->connection->postReceipt(this, event); + if (messageEvents().empty()) + return; + + QString prevLastReadId = lastReadEvent(connection()->user()); + setLastReadEvent(connection()->user(), (*--end)->id()); + + // We shouldn't send read receipts for messages from the local user - so + // shift back (if necessary) to the nearest message not from the local user + // or the so far last read message, whichever comes first. + for (; (*end)->id() != prevLastReadId; --end) + { + if ((*end)->senderId() != connection()->userId()) + { + d->connection->postReceipt(this, (*end)); + break; + } + if (end == messageEvents().begin()) + break; + } +} + +void Room::markMessagesAsRead() +{ + markMessagesAsRead(messageEvents().end()); } QString Room::lastReadEvent(User* user) diff --git a/room.h b/room.h index 3727af0e..049efeb6 100644 --- a/room.h +++ b/room.h @@ -69,9 +69,22 @@ namespace QMatrixClient Q_INVOKABLE void updateData(SyncRoomData& data ); Q_INVOKABLE void setJoinState( JoinState state ); - Q_INVOKABLE void setLastReadEvent(User* user, QString eventId); - Q_INVOKABLE void markMessageAsRead( Event* event ); Q_INVOKABLE QString lastReadEvent(User* user); + Q_INVOKABLE void setLastReadEvent(User* user, QString eventId); + /** + * @brief Mark the message at the iterator as read + * + * Marks the message at the iterator as read; also posts a read + * receipt to the server either for this message or, if it's from + * the local user, for the nearest non-local message before. + */ + Q_INVOKABLE void markMessagesAsRead(Timeline::const_iterator iter); + /** + * @brief Mark the most recent message in the timeline as read + * + * This effectively marks everything in the room as read. + */ + Q_INVOKABLE void markMessagesAsRead(); Q_INVOKABLE int notificationCount() const; Q_INVOKABLE void resetNotificationCount(); -- cgit v1.2.3 From 06a1131081e4f49a91f40ce00227104ebe6cd8fd Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 23 Oct 2016 08:43:22 +0900 Subject: A couple of fixes according to the PR review Room::markMessagesAsRead: use the iterator to the message, not after the message. Room::setLastReadEvent: moved to protected --- room.cpp | 18 ++++++++---------- room.h | 5 +++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/room.cpp b/room.cpp index 674f0343..df230e38 100644 --- a/room.cpp +++ b/room.cpp @@ -169,32 +169,30 @@ void Room::setLastReadEvent(User* user, QString eventId) emit lastReadEventChanged(user); } -void Room::markMessagesAsRead(Timeline::const_iterator end) +void Room::markMessagesAsRead(Timeline::const_iterator last) { - if (messageEvents().empty()) - return; - QString prevLastReadId = lastReadEvent(connection()->user()); - setLastReadEvent(connection()->user(), (*--end)->id()); + setLastReadEvent(connection()->user(), (*last)->id()); // We shouldn't send read receipts for messages from the local user - so // shift back (if necessary) to the nearest message not from the local user // or the so far last read message, whichever comes first. - for (; (*end)->id() != prevLastReadId; --end) + for (; (*last)->id() != prevLastReadId; --last) { - if ((*end)->senderId() != connection()->userId()) + if ((*last)->senderId() != connection()->userId()) { - d->connection->postReceipt(this, (*end)); + d->connection->postReceipt(this, (*last)); break; } - if (end == messageEvents().begin()) + if (last == messageEvents().begin()) break; } } void Room::markMessagesAsRead() { - markMessagesAsRead(messageEvents().end()); + if (!messageEvents().empty()) + markMessagesAsRead(messageEvents().end() - 1); } QString Room::lastReadEvent(User* user) diff --git a/room.h b/room.h index 049efeb6..9c745199 100644 --- a/room.h +++ b/room.h @@ -70,7 +70,6 @@ namespace QMatrixClient Q_INVOKABLE void setJoinState( JoinState state ); Q_INVOKABLE QString lastReadEvent(User* user); - Q_INVOKABLE void setLastReadEvent(User* user, QString eventId); /** * @brief Mark the message at the iterator as read * @@ -78,7 +77,7 @@ namespace QMatrixClient * receipt to the server either for this message or, if it's from * the local user, for the nearest non-local message before. */ - Q_INVOKABLE void markMessagesAsRead(Timeline::const_iterator iter); + Q_INVOKABLE void markMessagesAsRead(Timeline::const_iterator last); /** * @brief Mark the most recent message in the timeline as read * @@ -125,6 +124,8 @@ namespace QMatrixClient virtual void processStateEvents(const Events& events); virtual void processEphemeralEvent(Event* event); + void setLastReadEvent(User* user, QString eventId); + private: class Private; Private* d; -- cgit v1.2.3 From caaddebae6acf1a1c5281a5beba43782a8c899fc Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 23 Oct 2016 19:08:41 +0900 Subject: Room: Don't let the read marker (of any user) get back, only forward --- room.cpp | 25 +++++++++++++++++++++++-- room.h | 4 +++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/room.cpp b/room.cpp index df230e38..c2a1ce70 100644 --- a/room.cpp +++ b/room.cpp @@ -169,10 +169,31 @@ void Room::setLastReadEvent(User* user, QString eventId) emit lastReadEventChanged(user); } +bool Room::promoteReadMarker(User* user, QString eventId) +{ + // Check that the new read event is not before the previously set - only + // allow the read marker to move down the timeline, not up. + QString prevLastReadId = lastReadEvent(user); + // Older Qt doesn't provide rbegin()/rend() for Qt containers + for (auto it = messageEvents().end(); it != messageEvents().begin();) + { + --it; + if (prevLastReadId == (*it)->id()) + return false; + if (eventId == (*it)->id()) + { + setLastReadEvent(user, eventId); + return true; + } + } + return false; +} + void Room::markMessagesAsRead(Timeline::const_iterator last) { QString prevLastReadId = lastReadEvent(connection()->user()); - setLastReadEvent(connection()->user(), (*last)->id()); + if ( !promoteReadMarker(connection()->user(), (*last)->id()) ) + return; // We shouldn't send read receipts for messages from the local user - so // shift back (if necessary) to the nearest message not from the local user @@ -516,7 +537,7 @@ void Room::processEphemeralEvent(Event* event) for( const Receipt& r: receipts ) { if (auto m = d->member(r.userId)) - setLastReadEvent(m, eventId); + promoteReadMarker(m, eventId); } } } diff --git a/room.h b/room.h index 9c745199..fb3212e9 100644 --- a/room.h +++ b/room.h @@ -124,7 +124,7 @@ namespace QMatrixClient virtual void processStateEvents(const Events& events); virtual void processEphemeralEvent(Event* event); - void setLastReadEvent(User* user, QString eventId); + bool promoteReadMarker(User* user, QString eventId); private: class Private; @@ -132,6 +132,8 @@ namespace QMatrixClient void addNewMessageEvents(const Events& events); void addHistoricalMessageEvents(const Events& events); + + void setLastReadEvent(User* user, QString eventId); }; } -- cgit v1.2.3