From d4ea7e251b8d22173695a9ec7b572c4d208b0c53 Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Thu, 9 May 2024 05:55:11 +0000 Subject: [PATCH] narrow: Fix browser back not navigation to previous selected message. Reproducer: * Go to a stream narrow. * Scroll up to select a previous message. * Click on a different stream in the left sidebar. * Click browser back button, you land at first unread message instead of your selected message. Fixed by updating the hash before we render the new message list. --- web/src/narrow.js | 23 ++++++++++++++--------- web/tests/narrow_activate.test.js | 4 ++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/web/src/narrow.js b/web/src/narrow.js index 7c28b2ffa0..eeba23f186 100644 --- a/web/src/narrow.js +++ b/web/src/narrow.js @@ -466,12 +466,25 @@ export function activate(raw_terms, opts) { } assert(msg_list !== undefined); + // Put the narrow terms in the URL fragment/hash. + // + // opts.change_hash will be false when the URL fragment was + // the source of this narrow, and the fragment was not a link to + // a specific target message ID that has been moved. + // + // This needs to be called at the same time as updating the + // current message list so that we don't need to think about + // bugs related to the URL fragment/hash being desynced from + // mesasge_lists.current. + if (opts.change_hash) { + save_narrow(terms, opts.trigger); + } + // Show the new set of messages. It is important to set message_lists.current to // the view right as it's being shown, because we rely on message_lists.current // being shown for deciding when to condense messages. // From here on down, any calls to the narrow_state API will // reflect the requested narrow. - message_lists.update_current_message_list(msg_list); let select_immediately; @@ -574,14 +587,6 @@ export function activate(raw_terms, opts) { }); } - // Put the narrow terms in the URL fragment. - // Disabled when the URL fragment was the source - // of this narrow, but not if the fragment had - // a target message ID that has been moved. - if (opts.change_hash) { - save_narrow(terms, opts.trigger); - } - handle_post_view_change(msg_list); unread_ui.update_unread_banner(); diff --git a/web/tests/narrow_activate.test.js b/web/tests/narrow_activate.test.js index 7889cc03eb..e2ada83112 100644 --- a/web/tests/narrow_activate.test.js +++ b/web/tests/narrow_activate.test.js @@ -224,11 +224,11 @@ run_test("basics", ({override}) => { [message_feed_top_notices, "hide_top_of_narrow_notices"], [message_feed_loading, "hide_indicators"], [compose_banner, "clear_message_sent_banners"], + [message_viewport, "stop_auto_scrolling"], + [browser_history, "set_hash"], [compose_actions, "on_narrow"], [unread_ops, "process_visible"], [narrow_history, "save_narrow_state_and_flush"], - [message_viewport, "stop_auto_scrolling"], - [browser_history, "set_hash"], [typing_events, "render_notifications_for_narrow"], [compose_closed_ui, "update_buttons_for_stream_views"], [compose_closed_ui, "update_reply_recipient_label"],