message view: Don't mark selected message as read when narrowing.

On activating a narrow we always mark the first message it selects
as read, which is usually the first unread message. This behaviour
is unwanted.

The flow is as follows:
* On activating the narrow we call the `MessageList.select_id`
  method along with a `force_rerender` param.
  Here event = {
                 id: X,
                 previously_selected_id: -1,
		 ...
	       }
  where X = the first unread message id to be selected.

* Due to this `force_rerender` field, we first call the
  `rerender` method and then trigger our custom event handler
  (`message_selected.zulip`) which is responsible for marking
  message as read.

* However, due to this call path: `this.select_id` >
  `this.rerender` > `this.rerender_view` > `this.redo_selection`
  > `this.select_id`, the `message_selected.zulip` event gets
  triggered with event as = {
	                      id: X,
                              previously_selected_id: X,
		              ...
	                    }

* Thus the second selection event gets triggered before the
  first one having the current and previous id equal to "X".

This above `id = previously_selected_id` consition only occurs
for narrow activation (due to the `force_rerender` flag) or
when a message is selected again by clicking on it (message
is already read in this case).

For the All Messages view we have similar logic where, we pass
`mark_read` as false (which was previously unused) which skips
the "message_selected.zulip" event in `message_scroll.js`.

Work towards zulip#10886.

frontend_tests/node_tests/message_list.js

@@ -73,9 +73,45 @@ run_test("basics", (override) => {
 
     assert.deepEqual(list.all_messages(), messages);
 
-    override($, "Event", (ev) => {
+    let message_select_events = [];
+    override($, "Event", (ev, opts) => {
         assert.equal(ev, "message_selected.zulip");
+        message_select_events.push(opts);
     });
+
+    assert.equal(list.selected_id(), -1);
+    const stub = make_stub();
+    list.view.rerender_preserving_scrolltop = stub.f;
+    // When narrow initially renders in narrow.update_selection.
+    list.select_id(60, {
+        use_closest: true,
+        force_rerender: true,
+    });
+
+    const expected_message_select_events = [
+        {
+            id: 60,
+            previously_selected_id: 60,
+        },
+        {
+            id: 60,
+            previously_selected_id: -1,
+        },
+    ];
+    const actual_message_select_events = message_select_events.map((e) => ({
+        id: e.id,
+        previously_selected_id: e.previously_selected_id,
+    }));
+
+    // The render function is called which inturn calls selected_id,
+    // before the original message selection event takes place.
+    // Thus the event which contains the updated data is actually
+    // called first, instead of the original selected_id function call.
+    assert.equal(list.selected_id(), 60);
+    assert.equal(message_select_events.length, 2);
+    assert.deepEqual(actual_message_select_events, expected_message_select_events);
+    message_select_events = [];
+
     list.select_id(50);
 
     assert.equal(list.selected_id(), 50);

static/js/message_scroll.js

@@ -188,8 +188,12 @@ export function initialize() {
         if (event.id === -1) {
             return;
         }
+        if (!event.mark_read || event.id === event.previously_selected_id) {
+            // Do not mark selected message read on initial narrow load.
+            return;
+        }
 
-        if (event.mark_read && event.previously_selected_id !== -1) {
+        if (event.previously_selected_id !== -1) {
             // Mark messages between old pointer and new pointer as read
             let messages;
             if (event.id < event.previously_selected_id) {