Don't mark messages as read by visibility.

Trac #1428

(imported from commit b67e52d7434220e397ca20ffa49915de6633519c)

tools/jslint/check-all.js

@@ -44,7 +44,7 @@ var globals =
     + ' viewport restart_get_updates force_get_updates'
     + ' load_more_messages reset_load_more_status have_scrolled_away_from_top'
     + ' maybe_scroll_to_selected recenter_pointer_on_display suppress_scroll_pointer_update'
-    + ' process_visible_unread_messages message_range message_in_table process_loaded_for_unread'
+    + ' message_range message_in_table process_loaded_for_unread'
     + ' mark_all_as_read message_unread process_read_messages unread_in_current_view'
     + ' fast_forward_pointer recent_subjects unread_subjects'
     + ' add_message_metadata'

zephyr/static/js/notifications.js

@@ -30,10 +30,6 @@ exports.initialize = function () {
         $.each(notice_memory, function (index, notice_mem_entry) {
            notice_mem_entry.obj.cancel();
         });
-
-        // Update many places on the DOM to reflect unread
-        // counts.
-        process_visible_unread_messages();
     }).blur(function () {
         window_has_focus = false;
     });
@@ -268,18 +264,28 @@ function message_is_notifiable(message) {
               subs.receives_notifications(message.stream))));
 }
 
+function message_is_visible (vp, message) {
+    if (! notifications.window_has_focus()) {
+        return false;
+    }
+
+    var top = vp.visible_top;
+    var height = vp.visible_height;
+
+    var row = rows.get(message.id, current_msg_list.table_name);
+    var row_offset = row.offset();
+    var row_height = row.height();
+    // Very tall messages are visible once we've gotten past them
+    return (row_height > height && row_offset.top > top) || within_viewport(row_offset, row_height);
+}
+
 exports.received_messages = function (messages) {
-    var i;
+    var vp = viewport.message_viewport_info();
 
     $.each(messages, function (index, message) {
-        // We send notifications for messages which the user has
+        if (!message_is_notifiable(message)) return;
-        // configured as notifiable, as long as they haven't been
+        if (message_is_visible(vp, message)) return;
-        // marked as read by process_visible_unread_messages
+
-        // (which occurs if the message arrived onscreen while the
-        // window had focus).
-        if (!(message_is_notifiable(message) && unread.message_unread(message))) {
-            return;
-        }
         if (page_params.desktop_notifications_enabled &&
             browser_desktop_notifications_on()) {
             process_notification({message: message, webkit_notify: true});

zephyr/static/js/stream_list.js

@@ -290,7 +290,6 @@ $(function () {
             }
             rebuild_recent_subjects(op_stream[0], subject);
         }
-        process_visible_unread_messages();
     });
 
     $(document).on('narrow_deactivated.zephyr', function (event) {

zephyr/static/js/ui.js

@@ -822,11 +822,6 @@ $(function () {
             } else if (!have_scrolled_away_from_top) {
                 have_scrolled_away_from_top = true;
             }
-            // When the window scrolls, it may cause some messages to
-            // enter the screen and become read.  Calling
-            // process_visible_unread_messages will update necessary
-            // data structures and DOM elements.
-            setTimeout(process_visible_unread_messages, 0);
         }
     }
 
@@ -836,10 +831,6 @@ $(function () {
         scroll_timer = setTimeout(scroll_finished, 100);
     }
 
-    $(window).scroll(function () {
-        process_visible_unread_messages();
-    });
-
     $(window).scroll($.throttle(50, function (e) {
         scroll_finish();
     }));

zephyr/static/js/zephyr.js

@@ -345,68 +345,6 @@ function process_read_messages(messages) {
     update_unread_counts();
 }
 
-// If we ever materially change the algorithm for this function, we
-// may need to update notifications.received_messages as well.
-function process_visible_unread_messages(update_cursor) {
-    // For any messages visible on the screen, make sure they have been marked
-    // as unread.
-    if (! notifications.window_has_focus()) {
-        return;
-    }
-
-    var selected = current_msg_list.selected_message();
-    var vp = viewport.message_viewport_info();
-    var top = vp.visible_top;
-    var height = vp.visible_height;
-
-    // Being simplistic about this, the smallest message is 30 px high.
-    var selected_row = rows.get(current_msg_list.selected_id(), current_msg_list.table_name);
-    var num_neighbors = Math.floor(height / 30);
-    var candidates = $.merge(selected_row.prevAll("tr.message_row[zid]:lt(" + num_neighbors + ")"),
-                             selected_row.nextAll("tr.message_row[zid]:lt(" + num_neighbors + ")"));
-
-    var visible_messages = candidates.filter(function (idx, message) {
-        var row = $(message);
-        var row_offset = row.offset();
-        var row_height = row.height();
-        // Mark very tall messages as read once we've gotten past them
-        return (row_height > height && row_offset.top > top) || within_viewport(row_offset, row_height);
-    });
-
-    if (update_cursor) {
-        //save the state of respond_to_cursor, and reapply it every time we move the cursor
-        var probably_from_sent_message = respond_to_cursor;
-        $.map(visible_messages, function(msg) {
-            if ((current_msg_list.get(rows.id($(msg))).sent_by_me) &&
-                (current_msg_list.selected_message().id < rows.id($(msg)))) {
-                // every time we move the cursor, we set respond_to_cursor to false. This should only
-                // happen if the user initiated the cursor move, not us, so we reset it when processing
-                // these messages
-                current_msg_list.select_id(rows.id($(msg)), {then_scroll: true,
-                                                             from_scroll: true});
-                respond_to_cursor = probably_from_sent_message;
-            }
-        });
-    }
-
-    var mark_as_read = $.map(visible_messages, function(msg) {
-        var message = current_msg_list.get(rows.id($(msg)));
-        if (! unread.message_unread(message)) {
-            return undefined;
-        } else {
-            return message;
-        }
-    });
-
-    if (unread.message_unread(selected)) {
-        mark_as_read.push(selected);
-    }
-
-    if (mark_as_read.length > 0) {
-        process_read_messages(mark_as_read);
-    }
-}
-
 function mark_read_between(msg_list, start_id, end_id) {
     var mark_as_read = [];
     $.each(message_range(msg_list, start_id, end_id),
@@ -750,7 +688,6 @@ function maybe_add_narrowed_messages(messages, msg_list) {
 
             new_messages = $.map(new_messages, add_message_metadata);
             add_messages(new_messages, msg_list);
-            process_visible_unread_messages();
             compose.update_faded_messages();
         },
         error: function (xhr) {
@@ -904,19 +841,32 @@ function get_updates_success(data) {
             }
         }
 
-        // notifications.received_messages uses values set by
+        if (narrow.narrowed_by_reply()) {
-        // process_visible_unread_messages and thus must
+            // If you send a message when narrowed to a recipient, move the
-        // be called after it
+            // pointer to it.
+
+            // Save the state of respond_to_cursor, and reapply it every time
+            // we move the cursor.
+            var saved_respond_to_cursor = respond_to_cursor;
+
             var i;
-        var update_cursor = false;
+            var selected_id = current_msg_list.selected_message().id;
-        // check if we need to update the cursor, and do so if needed.
+
-        for (i = 0; i < messages.length; i++) {
+            // Iterate backwards to find the last message sent_by_me, stopping at
-            if (messages[i].sent_by_me && narrow.narrowed_by_reply()) {
+            // the pointer position.
-                update_cursor = true;
+            for (i = messages.length-1; i>=0; i--){
+                if (messages[i].id <= selected_id) break;
+                if (messages[i].sent_by_me) {
+                    // If this is a reply we just sent, advance the pointer to it.
+                    current_msg_list.select_id(messages[i].id, {then_scroll: true,
+                                                                from_scroll: true});
+                    break;
                 }
             }
 
-        process_visible_unread_messages(update_cursor);
+            respond_to_cursor = saved_respond_to_cursor;
+        }
+
         notifications.received_messages(messages);
         compose.update_faded_messages();
         stream_list.update_streams_sidebar();

zephyr/tests/frontend/tests/06-unread.js

@@ -4,8 +4,7 @@ var common = require('../common.js').common;
 // other tests send a message before this one is run,
 // changing the unread counts. uncomment line 43 to run
 // standalone
-// Silence jslint errors about the "process_visible_unread_messages" global
+// Silence jslint errors about the global
-/*global process_visible_unread_messages: true */
 /*global keep_pointer_in_view: true */
 
 function send_with_content(content) {
@@ -79,8 +78,7 @@ casper.then(function () {
         // Changing the scroll position in phantomjs doesn't seem to trigger on-scroll
         // handlers, so unread messages are not handled
         casper.page.scrollPosition = {top: ypos, left: 0};
-        casper.evaluate(function () { keep_pointer_in_view();
+        casper.evaluate(function () { keep_pointer_in_view(); });
-                                      process_visible_unread_messages(); });
     }
     var i = 0;
     scroll_to(0);