activity: Don't read new messages without user activity.

We have a bug where we can mark messages as read as a result of a
desktop left open with the Zulip window focused. To avoid it,
we only mark new messages as read if there is some user activity.

Note that we scroll to bring new messages into view which can
mark them as read.

web/src/activity.ts

@@ -58,8 +58,24 @@ export let client_is_active = document.hasFocus();
 // server-initiated reload as user activity.
 export let new_user_input = true;
 
+export let received_new_messages = false;
+
+type UserInputHook = () => void;
+const on_new_user_input_hooks: UserInputHook[] = [];
+
+export function register_on_new_user_input_hook(hook: UserInputHook): void {
+    on_new_user_input_hooks.push(hook);
+}
+
+export function set_received_new_messages(value: boolean): void {
+    received_new_messages = value;
+}
+
 export function set_new_user_input(value: boolean): void {
     new_user_input = value;
+    for (const hook of on_new_user_input_hooks) {
+        hook();
+    }
 }
 
 export function clear_for_testing(): void {

web/src/message_events.js

@@ -2,6 +2,7 @@ import $ from "jquery";
 import _ from "lodash";
 import assert from "minimalistic-assert";
 
+import * as activity from "./activity";
 import * as alert_words from "./alert_words";
 import * as channel from "./channel";
 import * as compose_fade from "./compose_fade";
@@ -35,7 +36,6 @@ import * as stream_list from "./stream_list";
 import * as stream_topic_history from "./stream_topic_history";
 import * as sub_store from "./sub_store";
 import * as unread from "./unread";
-import * as unread_ops from "./unread_ops";
 import * as unread_ui from "./unread_ui";
 import * as util from "./util";
 
@@ -274,7 +274,7 @@ export function insert_new_messages(messages, sent_by_this_client, deliver_local
         messages.map((message) => echo.track_local_message(message));
     }
 
-    unread_ops.process_visible();
+    activity.set_received_new_messages(true);
     message_notifications.received_messages(messages);
     stream_list.update_streams_sidebar();
     pm_list.update_private_messages();

web/src/ui_init.js

@@ -634,6 +634,16 @@ export function initialize_everything(state_data) {
 
     initialize_unread_ui();
     activity.initialize();
+    activity.register_on_new_user_input_hook(() => {
+        // Instead of marking new messages as read immediately when bottom
+        // of feed is visible, we wait for user input to mark them as read.
+        // This is to prevent marking messages as read unintentionally,
+        // especially when user is away from screen and the window is focused.
+        if (activity.received_new_messages && activity.new_user_input) {
+            unread_ops.process_visible();
+            activity.set_received_new_messages(false);
+        }
+    });
     activity_ui.initialize({
         narrow_by_email(email) {
             message_view.show(

web/tests/activity.test.js

@@ -916,6 +916,10 @@ test("electron_bridge", ({override_rewire}) => {
         activity.mark_client_active();
         assert.equal(activity.compute_active_status(), "active");
     });
+
+    assert.ok(!activity.received_new_messages);
+    activity.set_received_new_messages(true);
+    assert.ok(activity.received_new_messages);
 });
 
 test("test_send_or_receive_no_presence_for_spectator", () => {

web/tests/example5.test.js

@@ -27,8 +27,8 @@ const message_notifications = mock_esm("../src/message_notifications");
 const message_util = mock_esm("../src/message_util");
 const pm_list = mock_esm("../src/pm_list");
 const stream_list = mock_esm("../src/stream_list");
-const unread_ops = mock_esm("../src/unread_ops");
 const unread_ui = mock_esm("../src/unread_ui");
+const activity = mock_esm("../src/activity");
 
 message_lists.current = {
     data: {
@@ -105,8 +105,8 @@ run_test("insert_message", ({override}) => {
     helper.redirect(message_notifications, "received_messages");
     helper.redirect(message_util, "add_new_messages");
     helper.redirect(stream_list, "update_streams_sidebar");
-    helper.redirect(unread_ops, "process_visible");
     helper.redirect(unread_ui, "update_unread_counts");
+    helper.redirect(activity, "set_received_new_messages");
 
     message_events.insert_new_messages([new_message]);
 
@@ -118,7 +118,7 @@ run_test("insert_message", ({override}) => {
         [direct_message_group_data, "process_loaded_messages"],
         [message_util, "add_new_messages"],
         [unread_ui, "update_unread_counts"],
-        [unread_ops, "process_visible"],
+        [activity, "set_received_new_messages"],
         [message_notifications, "received_messages"],
         [stream_list, "update_streams_sidebar"],
     ]);

web/tests/hotkey.test.js

@@ -34,6 +34,7 @@ set_global("document", {
 });
 
 const activity_ui = mock_esm("../src/activity_ui");
+const activity = zrequire("../src/activity");
 const browser_history = mock_esm("../src/browser_history");
 const compose_actions = mock_esm("../src/compose_actions");
 const compose_reply = mock_esm("../src/compose_reply");
@@ -543,3 +544,13 @@ run_test("motion_keys", () => {
     delete overlays.any_active;
     delete overlays.drafts_open;
 });
+
+run_test("test new user input hook called", () => {
+    let hook_called = false;
+    activity.register_on_new_user_input_hook(() => {
+        hook_called = true;
+    });
+
+    hotkey.process_keydown({which: "S".codePointAt(0)});
+    assert.ok(hook_called);
+});