hashchange: Never set empty hash `#` in URL.

Setting empty hash `#` scrolls user to the top of message feed if
done via `window.location.hash` or using browser back / forward button.

To avoid this, we set don't set `hash` after org URL for default view
when user uses `escape` key or clicks on org logo.

In other situations, we explicitly set the hash of the view.

web/src/browser_history.js

@@ -1,6 +1,9 @@
+// TODO: Rewrite this module to use history.pushState.
+
 import * as blueslip from "./blueslip";
 import * as hash_util from "./hash_util";
 import * as ui_util from "./ui_util";
+import {user_settings} from "./user_settings";
 
 export const state = {
     is_internal_change: false,
@@ -13,7 +16,7 @@ export const state = {
     // hashes are changed without calling `hashchanged` in many ways.
     spectator_old_hash: hash_util.is_spectator_compatible(window.location.hash)
         ? window.location.hash
-        : "#",
+        : `#${user_settings.default_view}`,
 };
 
 export function clear_for_testing() {
@@ -73,7 +76,7 @@ export function update(new_hash) {
 export function exit_overlay() {
     if (hash_util.is_overlay_hash(window.location.hash) && !state.changing_hash) {
         ui_util.blur_active_element();
-        const new_hash = state.hash_before_overlay || "#";
+        const new_hash = state.hash_before_overlay || `#${user_settings.default_view}`;
         update(new_hash);
     }
 }

web/src/click_handlers.js

@@ -18,6 +18,7 @@ import {media_breakpoints_num} from "./css_variables";
 import * as dark_theme from "./dark_theme";
 import * as emoji_picker from "./emoji_picker";
 import * as hash_util from "./hash_util";
+import * as hashchange from "./hashchange";
 import * as message_edit from "./message_edit";
 import * as message_flags from "./message_flags";
 import * as message_lists from "./message_lists";
@@ -859,6 +860,13 @@ export function initialize() {
         message_lists.update_recipient_bar_background_color();
     });
 
+    $("body").on("click", "#header-container .brand", (e) => {
+        e.preventDefault();
+        e.stopPropagation();
+
+        hashchange.set_hash_to_default_view();
+    });
+
     // MAIN CLICK HANDLER
 
     $(document).on("click", (e) => {

web/src/hashchange.js

@@ -35,7 +35,7 @@ import {user_settings} from "./user_settings";
 function get_full_url(hash) {
     const location = window.location;
 
-    if (hash === "" || hash.charAt(0) !== "#") {
+    if (hash.charAt(0) !== "#" && hash !== "") {
         hash = "#" + hash;
     }
 
@@ -71,7 +71,14 @@ function set_hash(hash) {
             }
         }
     } else {
-        blueslip.warn("browser does not support pushState");
+        // pushState has 97% global support according to caniuse. So, we will ideally never reach here.
+        // TODO: Delete this case if we don't see any error reports in a while.
+        if (hash === "" || hash === "#") {
+            // Setting empty hash here would scroll to the top.
+            hash = user_settings.default_view;
+        }
+
+        blueslip.error("browser does not support pushState");
         window.location.hash = hash;
     }
 }
@@ -112,7 +119,20 @@ function show_all_message_view() {
 }
 
 export function set_hash_to_default_view() {
-    window.location.hash = "";
+    let default_view_hash = `#${user_settings.default_view}`;
+    if (default_view_hash === "#recent_topics") {
+        default_view_hash = "#recent";
+    }
+
+    if (window.location.hash !== default_view_hash) {
+        // We want to set URL with no hash here. It is not possible
+        // to do so with `window.location.hash` since it will set an empty
+        // hash. So, we use `pushState` which simply updates the current URL
+        // but doesn't trigger `hashchange`. So, we trigger hashchange directly
+        // here to let it handle the whole rendering process for us.
+        set_hash("");
+        hashchanged(false);
+    }
 }
 
 function show_default_view() {
@@ -239,7 +259,7 @@ function do_hashchange_overlay(old_hash) {
         return;
     }
 
-    const coming_from_overlay = hash_util.is_overlay_hash(old_hash || "#");
+    const coming_from_overlay = hash_util.is_overlay_hash(old_hash);
 
     if ((base === "settings" || base === "organization") && !section) {
         let settings_panel_object = settings_panel_menu.normal_settings;

web/src/server_events_dispatch.js

@@ -6,6 +6,7 @@ import * as alert_words_ui from "./alert_words_ui";
 import * as attachments_ui from "./attachments_ui";
 import * as blueslip from "./blueslip";
 import * as bot_data from "./bot_data";
+import * as browser_history from "./browser_history";
 import {buddy_list} from "./buddy_list";
 import * as compose from "./compose";
 import * as compose_fade from "./compose_fade";
@@ -677,6 +678,7 @@ export function dispatch_normal_event(event) {
                 "send_read_receipts",
             ];
 
+            const original_default_view = user_settings.default_view;
             if (user_display_settings.includes(event.property)) {
                 user_settings[event.property] = event.value;
             }
@@ -689,6 +691,20 @@ export function dispatch_normal_event(event) {
                 // present in the backend/Jinja2 templates.
                 settings_display.set_default_language_name(event.language_name);
             }
+            if (
+                event.property === "default_view" && // If current hash is empty (default view), and the
+                // user changes the default view while in settings,
+                // then going back to an empty hash on closing the
+                // overlay will not match the view currently displayed
+                // under settings, so we set the hash to the previous
+                // value of the default view.
+                !browser_history.state.hash_before_overlay &&
+                overlays.settings_open()
+            ) {
+                browser_history.state.hash_before_overlay =
+                    "#" +
+                    (original_default_view === "recent_topics" ? "recent" : original_default_view);
+            }
             if (event.property === "twenty_four_hour_time") {
                 // Rerender the whole message list UI
                 for (const msg_list of message_lists.all_rendered_message_lists()) {

web/templates/navbar.hbs

@@ -3,7 +3,7 @@
     <div class="top_navbar_full_width">
         <nav class="header-main" id="top_navbar">
             <div class="column-left">
-                <a class="brand no-style" href="#">
+                <a href="" class="brand no-style">
                     <img id="realm-logo" src="" alt="" class="nav-logo no-drag"/>
                 </a>
             </div>

web/tests/browser_history.test.js

@@ -6,7 +6,9 @@ const {zrequire} = require("./lib/namespace");
 const {make_stub} = require("./lib/stub");
 const {run_test} = require("./lib/test");
 const blueslip = require("./lib/zblueslip");
+const {user_settings} = require("./lib/zpage_params");
 
+user_settings.default_view = "recent";
 window.location.hash = "#bogus";
 
 const browser_history = zrequire("browser_history");
@@ -73,5 +75,5 @@ test("web-public view hash restore", () => {
     browser_history.update(new_hash);
     assert.equal(window.location.hash, new_hash);
     browser_history.return_to_web_public_hash();
-    assert.equal(window.location.hash, "");
+    assert.equal(window.location.hash, "#recent");
 });

web/tests/dispatch.test.js

@@ -816,6 +816,7 @@ run_test("user_settings", ({override}) => {
     let event = event_fixtures.user_settings__default_language;
     user_settings.default_language = "en";
     override(settings_display, "update_page", noop);
+    override(overlays, "settings_open", () => true);
     dispatch(event);
     assert_same(user_settings.default_language, "fr");
 

web/tests/hashchange.test.js

@@ -342,7 +342,7 @@ run_test("save_narrow", ({override}) => {
 
     let operators = [{operator: "is", operand: "dm"}];
 
-    blueslip.expect("warn", "browser does not support pushState");
+    blueslip.expect("error", "browser does not support pushState");
     hashchange.save_narrow(operators);
 
     helper.assert_events([[message_viewport, "stop_auto_scrolling"]]);