From c1f12e3f8aac387260e86e896dc188ed6d26024e Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Wed, 11 Oct 2017 15:49:25 -0700 Subject: [PATCH] scrolling: Fix out-of-order bug in the message list. The issue has a lot of extra details, but in short, if several messages were sent at very close to the same time, it's possible that the event queues will receive the "new message" events out-of-order. This, in turn, could cause `get_events` to return an incorrectly sorted block of messages. These would then be passed into `message_list.add_messages`, which doesn't handle that sort of unsorted situation correctly (in short, the `self.first.id()` comparison checks are not accurate for that situation, since we don't update the boundaries after the first messages is processed). The end result of this bug was that it was possible for the message list to be out-of-order, which in turn would cause exceptions when scrolling with the mouse. Fixes #6948. --- static/js/message_list.js | 6 +++++- static/js/server_events.js | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/static/js/message_list.js b/static/js/message_list.js index 655526d642..2dabd09b05 100644 --- a/static/js/message_list.js +++ b/static/js/message_list.js @@ -55,7 +55,11 @@ exports.MessageList.prototype = { return; } - // Put messages in correct order on either side of the message list + // Put messages in correct order on either side of the + // message list. This code path assumes that messages + // is a (1) sorted, and (2) consecutive block of + // messages that belong in this message list; those + // facts should be ensured by the caller. if (self.empty() || msg.id > self.last().id) { bottom_messages.push(msg); } else if (msg.id < self.first().id) { diff --git a/static/js/server_events.js b/static/js/server_events.js index 9cd980cd92..358d4adb76 100644 --- a/static/js/server_events.js +++ b/static/js/server_events.js @@ -89,6 +89,10 @@ function get_events_success(events) { }); if (messages.length !== 0) { + // Sort by ID, so that if we get multiple messages back from + // the server out-of-order, we'll still end up with our + // message lists in order. + messages = _.sortBy(messages, 'id'); try { messages = echo.process_from_server(messages); message_events.insert_new_messages(messages);