diff --git a/web/src/message_list_view.js b/web/src/message_list_view.js index a729fac4b8..a1351d1eef 100644 --- a/web/src/message_list_view.js +++ b/web/src/message_list_view.js @@ -1451,6 +1451,15 @@ export class MessageListView { ); } + is_start_rendered() { + // Used as a helper in checks for whether a given scroll + // position is actually the very start of this view. It could + // fail to be for two reasons: Either some older messages are + // not rendered due to a render window, or we haven't finished + // fetching the oldest messages for this view from the server. + return this._render_win_start === 0 && this.list.data.fetch_status.has_found_oldest(); + } + get_row(id) { const $row = this._rows.get(id); diff --git a/web/src/navigate.js b/web/src/navigate.js index 1bfafbda3f..052a8cd7ea 100644 --- a/web/src/navigate.js +++ b/web/src/navigate.js @@ -100,7 +100,11 @@ export function page_down_the_right_amount() { } export function page_up() { - if (message_viewport.at_rendered_top() && !message_lists.current.visibly_empty()) { + if ( + message_viewport.at_rendered_top() && + !message_lists.current.visibly_empty() && + message_lists.current.view.is_start_rendered() + ) { message_lists.current.select_id(message_lists.current.first().id, {then_scroll: false}); } else { page_up_the_right_amount(); @@ -108,7 +112,11 @@ export function page_up() { } export function page_down() { - if (message_viewport.at_rendered_bottom() && !message_lists.current.visibly_empty()) { + if ( + message_viewport.at_rendered_bottom() && + !message_lists.current.visibly_empty() && + message_lists.current.view.is_end_rendered() + ) { message_lists.current.select_id(message_lists.current.last().id, {then_scroll: false}); unread_ops.process_visible(); } else { diff --git a/web/tests/example7.test.js b/web/tests/example7.test.js index ac3376ec16..5937d9b4d0 100644 --- a/web/tests/example7.test.js +++ b/web/tests/example7.test.js @@ -125,6 +125,7 @@ run_test("unread_ops", ({override}) => { // data setup). override(message_lists.current, "can_mark_messages_read", () => can_mark_messages_read); override(message_lists.current, "has_unread_messages", () => true); + override(message_lists.current.view, "is_end_rendered", () => true); // First, test for a message list that cannot read messages. can_mark_messages_read = false; @@ -134,6 +135,12 @@ run_test("unread_ops", ({override}) => { // Now flip the boolean, and get to the main thing we are testing. can_mark_messages_read = true; + // Don't mark messages as read until all messages in the narrow are fetched and rendered. + override(message_lists.current.view, "is_end_rendered", () => false); + unread_ops.process_visible(); + assert.deepEqual(channel_post_opts, undefined); + + override(message_lists.current.view, "is_end_rendered", () => true); unread_ops.process_visible(); // The most important side effect of the above call is that