From 0da547a239e652e1c1cf296d4bac2e92b97ceea6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Nov 2015 13:39:03 +0000 Subject: [PATCH 1/4] Implicit read receipts * Inject implicit read receipts into the timeline * Twiddle local echo a bit to make the implicit receipts match the various different stages of local echo. --- lib/client.js | 9 ++++++--- lib/models/room.js | 42 +++++++++++++++++++++++++++++++++++++++++- lib/utils.js | 6 ++++-- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/lib/client.js b/lib/client.js index 69f288d63..2f7c4219d 100644 --- a/lib/client.js +++ b/lib/client.js @@ -946,6 +946,9 @@ function _sendEvent(client, room, event, callback) { // the fake event we made above. If we don't find it, we're still // waiting on the real event and so should assign the fake event // with the real event_id for matching later. + + // FIXME: This manipulation of the room should probably be done + // inside the room class, not by the client. var matchingEvent = utils.findElement(room.timeline, function(ev) { return ev.getId() === eventId; }, true); @@ -958,13 +961,13 @@ function _sendEvent(client, room, event, callback) { matchingEvent.event.content = event.event.content; matchingEvent.event.type = event.event.type; } - utils.removeElement(room.timeline, function(ev) { - return ev.getId() === event.getId(); - }, true); + room.removeEvents([event.getId()]); } else { + room.removeEvents([event.getId()]); event.event.event_id = res.event_id; event.status = null; + room.addEventsToTimeline([event]); } } diff --git a/lib/models/room.js b/lib/models/room.js index 724ac559f..3abf8f580 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -194,7 +194,26 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline) { else { this.timeline.push(events[i]); } - this.emit("Room.timeline", events[i], this, Boolean(toStartOfTimeline)); + + // synthesize and inject implicit read receipts + // Done after adding the event because otherwise the app would get a read receipt + // pointing to an event that wasn't yet in the timeline + + // This is really ugly because JS has no way to express an object literal where the + // name of a key comes from an expression + if (events[i].sender) { + var fakeReceipt = {content: {}}; + fakeReceipt.content[events[i].getId()] = { + 'm.read': { + } + }; + fakeReceipt.content[events[i].getId()]['m.read'][events[i].sender.userId] = { + ts: events[i].getTs() + }; + this.addReceipt(new MatrixEvent(fakeReceipt)); + } + + this.emit("Room.timeline", events[i], this, Boolean(toStartOfTimeline), false); } }; @@ -260,6 +279,26 @@ Room.prototype.addEvents = function(events, duplicateStrategy) { } }; +/** + * Removes events from this room. + * @param {String} event_ids A list of event_ids to remove. + */ +Room.prototype.removeEvents = function(event_ids) { + for (var i = 0; i < event_ids.length; ++i) { + // NB. we supply reverse to search from the end, + // on the assumption that recents events are much + // more likley to be removed than older ones. + var removed = utils.removeElement( + this.timeline, function(e) { + return e.getId() == event_ids[i]; + }, true + ); + if (removed !== false) { + this.emit("Room.timeline", removed, this, undefined, true); + } + } +}; + /** * Recalculate various aspects of the room, including the room name and * room summary. Call this any time the room's current state is modified. @@ -513,6 +552,7 @@ module.exports = Room; * @param {MatrixEvent} event The matrix event which caused this event to fire. * @param {Room} room The room whose Room.timeline was updated. * @param {boolean} toStartOfTimeline True if this event was added to the start + * @param {boolean} removed True if this event has just been removed from the timeline * (beginning; oldest) of the timeline e.g. due to pagination. * @example * matrixClient.on("Room.timeline", function(event, room, toStartOfTimeline){ diff --git a/lib/utils.js b/lib/utils.js index a18a1dc21..141aef734 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -155,16 +155,18 @@ module.exports.removeElement = function(array, fn, reverse) { if (reverse) { for (i = array.length - 1; i >= 0; i--) { if (fn(array[i], i, array)) { + var removed = array[i]; array.splice(i, 1); - return true; + return removed; } } } else { for (i = 0; i < array.length; i++) { if (fn(array[i], i, array)) { + var removed = array[i]; array.splice(i, 1); - return true; + return removed; } } } From ad80d4f0598c8acdc118a68deb1d8c05f57f88f2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Nov 2015 13:57:21 +0000 Subject: [PATCH 2/4] fix lint errors --- lib/models/room.js | 13 ++++++++----- lib/utils.js | 5 +++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index 3abf8f580..eb6ff3ae9 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -199,8 +199,8 @@ Room.prototype.addEventsToTimeline = function(events, toStartOfTimeline) { // Done after adding the event because otherwise the app would get a read receipt // pointing to an event that wasn't yet in the timeline - // This is really ugly because JS has no way to express an object literal where the - // name of a key comes from an expression + // This is really ugly because JS has no way to express an object literal + // where the name of a key comes from an expression if (events[i].sender) { var fakeReceipt = {content: {}}; fakeReceipt.content[events[i].getId()] = { @@ -284,14 +284,17 @@ Room.prototype.addEvents = function(events, duplicateStrategy) { * @param {String} event_ids A list of event_ids to remove. */ Room.prototype.removeEvents = function(event_ids) { + // avoids defining a function in the loop, which is a lint error + function eq(a, b) { + return a === b; + } + for (var i = 0; i < event_ids.length; ++i) { // NB. we supply reverse to search from the end, // on the assumption that recents events are much // more likley to be removed than older ones. var removed = utils.removeElement( - this.timeline, function(e) { - return e.getId() == event_ids[i]; - }, true + this.timeline, eq.bind(event_ids[i]), true ); if (removed !== false) { this.emit("Room.timeline", removed, this, undefined, true); diff --git a/lib/utils.js b/lib/utils.js index 141aef734..955903fff 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -152,10 +152,11 @@ module.exports.findElement = function(array, fn, reverse) { */ module.exports.removeElement = function(array, fn, reverse) { var i; + var removed; if (reverse) { for (i = array.length - 1; i >= 0; i--) { if (fn(array[i], i, array)) { - var removed = array[i]; + removed = array[i]; array.splice(i, 1); return removed; } @@ -164,7 +165,7 @@ module.exports.removeElement = function(array, fn, reverse) { else { for (i = 0; i < array.length; i++) { if (fn(array[i], i, array)) { - var removed = array[i]; + removed = array[i]; array.splice(i, 1); return removed; } From 856c34016d947618228c38d16abd508394848916 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Nov 2015 14:13:52 +0000 Subject: [PATCH 3/4] Fix event removal --- lib/models/room.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index eb6ff3ae9..0099ded81 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -285,17 +285,17 @@ Room.prototype.addEvents = function(events, duplicateStrategy) { */ Room.prototype.removeEvents = function(event_ids) { // avoids defining a function in the loop, which is a lint error - function eq(a, b) { - return a === b; - } - - for (var i = 0; i < event_ids.length; ++i) { + function remove_event_with_id(timeline, id) { // NB. we supply reverse to search from the end, // on the assumption that recents events are much // more likley to be removed than older ones. - var removed = utils.removeElement( - this.timeline, eq.bind(event_ids[i]), true - ); + return utils.removeElement(timeline, function(e) { + return e.getId() == id; + }, true); + } + + for (var i = 0; i < event_ids.length; ++i) { + var removed = remove_event_with_id(this.timeline, event_ids[i]); if (removed !== false) { this.emit("Room.timeline", removed, this, undefined, true); } From 483095c3da6bbb863006dcbd3af33a3227c8c7a2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Nov 2015 14:41:35 +0000 Subject: [PATCH 4/4] Fix PR comments --- lib/models/room.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/models/room.js b/lib/models/room.js index 0099ded81..2692455ab 100644 --- a/lib/models/room.js +++ b/lib/models/room.js @@ -285,7 +285,7 @@ Room.prototype.addEvents = function(events, duplicateStrategy) { */ Room.prototype.removeEvents = function(event_ids) { // avoids defining a function in the loop, which is a lint error - function remove_event_with_id(timeline, id) { + function removeEventWithId(timeline, id) { // NB. we supply reverse to search from the end, // on the assumption that recents events are much // more likley to be removed than older ones. @@ -295,8 +295,8 @@ Room.prototype.removeEvents = function(event_ids) { } for (var i = 0; i < event_ids.length; ++i) { - var removed = remove_event_with_id(this.timeline, event_ids[i]); - if (removed !== false) { + var removed = removeEventWithId(this.timeline, event_ids[i]); + if (removed) { this.emit("Room.timeline", removed, this, undefined, true); } }