From 564e91f3a8898db4432cf1e878d41cf97132c8af Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Fri, 6 Oct 2023 16:08:53 -0700 Subject: [PATCH] narrow: Cut import of hashchange. Signed-off-by: Anders Kaseorg --- tools/test-js-with-node | 1 + web/src/browser_history.ts | 51 +++++++++++++++++++++ web/src/hashchange.js | 73 ++----------------------------- web/src/narrow.js | 23 ++++++++-- web/src/reload_setup.js | 6 +-- web/tests/hashchange.test.js | 18 ++++---- web/tests/narrow_activate.test.js | 11 +++-- 7 files changed, 95 insertions(+), 88 deletions(-) diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 8d8cb83079..8d4ab4630c 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -61,6 +61,7 @@ EXEMPT_FILES = make_set( "web/src/billing/upgrade.ts", "web/src/blueslip.ts", "web/src/blueslip_stacktrace.ts", + "web/src/browser_history.ts", "web/src/click_handlers.js", "web/src/compose.js", "web/src/compose_actions.js", diff --git a/web/src/browser_history.ts b/web/src/browser_history.ts index 469d3bbf27..168f0fb43e 100644 --- a/web/src/browser_history.ts +++ b/web/src/browser_history.ts @@ -102,3 +102,54 @@ export function update_hash_internally_if_required(hash: string): void { export function return_to_web_public_hash(): void { window.location.hash = state.spectator_old_hash ?? `#${user_settings.default_view}`; } + +export function get_full_url(hash: string): string { + const location = window.location; + + if (hash.charAt(0) !== "#" && hash !== "") { + hash = "#" + hash; + } + + // IE returns pathname as undefined and missing the leading / + let pathname = location.pathname; + if (pathname === undefined) { + pathname = "/"; + } else if (pathname === "" || pathname.charAt(0) !== "/") { + pathname = "/" + pathname; + } + + // Build a full URL to not have same origin problems + const url = location.protocol + "//" + location.host + pathname + hash; + return url; +} + +export function set_hash(hash: string): void { + if (hash === window.location.hash) { + // Avoid adding duplicate entries in browser history. + return; + } + if (history.pushState) { + const url = get_full_url(hash); + try { + history.pushState(null, "", url); + update_web_public_hash(hash); + } catch (error) { + if (error instanceof TypeError) { + // The window has been destroyed and the history object has been marked dead, so cannot + // be updated. Silently do nothing, since there's nothing we can do. + } else { + throw error; + } + } + } else { + // 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; + } +} diff --git a/web/src/hashchange.js b/web/src/hashchange.js index f3e8730829..75d690665a 100644 --- a/web/src/hashchange.js +++ b/web/src/hashchange.js @@ -35,57 +35,6 @@ import {user_settings} from "./user_settings"; // Read https://zulip.readthedocs.io/en/latest/subsystems/hashchange-system.html // or locally: docs/subsystems/hashchange-system.md -function get_full_url(hash) { - const location = window.location; - - if (hash.charAt(0) !== "#" && hash !== "") { - hash = "#" + hash; - } - - // IE returns pathname as undefined and missing the leading / - let pathname = location.pathname; - if (pathname === undefined) { - pathname = "/"; - } else if (pathname === "" || pathname.charAt(0) !== "/") { - pathname = "/" + pathname; - } - - // Build a full URL to not have same origin problems - const url = location.protocol + "//" + location.host + pathname + hash; - return url; -} - -function set_hash(hash) { - if (hash === window.location.hash) { - // Avoid adding duplicate entries in browser history. - return; - } - if (history.pushState) { - const url = get_full_url(hash); - try { - history.pushState(null, null, url); - browser_history.update_web_public_hash(hash); - } catch (error) { - if (error instanceof TypeError) { - // The window has been destroyed and the history object has been marked dead, so cannot - // be updated. Silently do nothing, since there's nothing we can do. - } else { - throw error; - } - } - } else { - // 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; - } -} - function maybe_hide_recent_view() { if (recent_view_util.is_visible()) { recent_view_ui.hide(); @@ -102,22 +51,6 @@ function maybe_hide_inbox() { return false; } -export function changehash(newhash) { - if (browser_history.state.changing_hash) { - return; - } - message_viewport.stop_auto_scrolling(); - set_hash(newhash); -} - -export function save_narrow(operators) { - if (browser_history.state.changing_hash) { - return; - } - const new_hash = hash_util.operators_to_hash(operators); - changehash(new_hash); -} - function show_all_message_view() { const coming_from_recent_view = maybe_hide_recent_view(); const coming_from_inbox = maybe_hide_inbox(); @@ -140,7 +73,7 @@ export function set_hash_to_default_view() { // 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(""); + browser_history.set_hash(""); hashchanged(false); } } @@ -310,12 +243,12 @@ function do_hashchange_overlay(old_hash) { history.replaceState( null, "", - get_full_url(base + "/" + settings_panel_object.current_tab()), + browser_history.get_full_url(base + "/" + settings_panel_object.current_tab()), ); } if (base === "streams" && !section) { - history.replaceState(null, "", get_full_url("streams/subscribed")); + history.replaceState(null, "", browser_history.get_full_url("streams/subscribed")); } // Start by handling the specific case of going diff --git a/web/src/narrow.js b/web/src/narrow.js index 37c413ad54..5ae5e0f5a9 100644 --- a/web/src/narrow.js +++ b/web/src/narrow.js @@ -14,7 +14,7 @@ import * as compose_state from "./compose_state"; import * as condense from "./condense"; import {Filter} from "./filter"; import * as hash_parser from "./hash_parser"; -import * as hashchange from "./hashchange"; +import * as hash_util from "./hash_util"; import * as inbox_ui from "./inbox_ui"; import * as inbox_util from "./inbox_util"; import * as left_sidebar_navigation_area from "./left_sidebar_navigation_area"; @@ -28,6 +28,7 @@ import {MessageListData} from "./message_list_data"; import * as message_lists from "./message_lists"; import * as message_store from "./message_store"; import * as message_view_header from "./message_view_header"; +import * as message_viewport from "./message_viewport"; import * as narrow_banner from "./narrow_banner"; import * as narrow_history from "./narrow_history"; import * as narrow_state from "./narrow_state"; @@ -90,6 +91,22 @@ export function handle_middle_pane_transition() { } } +export function changehash(newhash) { + if (browser_history.state.changing_hash) { + return; + } + message_viewport.stop_auto_scrolling(); + browser_history.set_hash(newhash); +} + +export function save_narrow(operators) { + if (browser_history.state.changing_hash) { + return; + } + const new_hash = hash_util.operators_to_hash(operators); + changehash(new_hash); +} + export function activate(raw_operators, opts) { /* Main entry point for switching to a new view / message list. Note that for historical reasons related to the current @@ -479,7 +496,7 @@ export function activate(raw_operators, opts) { // Disabled when the URL fragment was the source // of this narrow. if (opts.change_hash) { - hashchange.save_narrow(operators); + save_narrow(operators); } handle_post_view_change(msg_list); @@ -1019,7 +1036,7 @@ export function deactivate(coming_from_all_messages = true, is_actively_scrollin reset_ui_state(); handle_middle_pane_transition(); - hashchange.save_narrow(); + save_narrow(); if (message_lists.current.selected_id() !== -1) { const preserve_pre_narrowing_screen_position = diff --git a/web/src/reload_setup.js b/web/src/reload_setup.js index 103e051837..59601a98d0 100644 --- a/web/src/reload_setup.js +++ b/web/src/reload_setup.js @@ -2,8 +2,8 @@ import * as activity from "./activity"; import * as blueslip from "./blueslip"; import * as compose from "./compose"; import * as compose_actions from "./compose_actions"; -import * as hashchange from "./hashchange"; import {localstorage} from "./localstorage"; +import * as narrow from "./narrow"; import {page_params} from "./page_params"; // Check if we're doing a compose-preserving reload. This must be @@ -27,7 +27,7 @@ export function initialize() { // exist, but be log it so that it's available for future // debugging if an exception happens later. blueslip.info("Invalid hash change reload token"); - hashchange.changehash(""); + narrow.changehash(""); return; } ls.remove(hash_fragment); @@ -87,5 +87,5 @@ export function initialize() { } activity.set_new_user_input(false); - hashchange.changehash(vars.oldhash); + narrow.changehash(vars.oldhash); } diff --git a/web/tests/hashchange.test.js b/web/tests/hashchange.test.js index 651f5da219..d9cbbce392 100644 --- a/web/tests/hashchange.test.js +++ b/web/tests/hashchange.test.js @@ -18,7 +18,6 @@ const admin = mock_esm("../src/admin"); const drafts_overlay_ui = mock_esm("../src/drafts_overlay_ui"); const info_overlay = mock_esm("../src/info_overlay"); const message_viewport = mock_esm("../src/message_viewport"); -const narrow = mock_esm("../src/narrow"); const overlays = mock_esm("../src/overlays"); const popovers = mock_esm("../src/popovers"); const recent_view_ui = mock_esm("../src/recent_view_ui"); @@ -32,6 +31,7 @@ const browser_history = zrequire("browser_history"); const people = zrequire("people"); const hash_util = zrequire("hash_util"); const hashchange = zrequire("hashchange"); +const narrow = zrequire("../src/narrow"); const stream_data = zrequire("stream_data"); run_test("operators_round_trip", () => { @@ -120,7 +120,7 @@ run_test("people_slugs", () => { assert.deepEqual(narrow, [{operator: "pm-with", operand: "alice@example.com", negated: false}]); }); -function test_helper({override, change_tab}) { +function test_helper({override, override_rewire, change_tab}) { let events = []; let narrow_terms; @@ -143,7 +143,7 @@ function test_helper({override, change_tab}) { stub(ui_report, "error"); if (change_tab) { - override(narrow, "activate", (terms) => { + override_rewire(narrow, "activate", (terms) => { narrow_terms = terms; events.push("narrow.activate"); }); @@ -164,11 +164,11 @@ function test_helper({override, change_tab}) { }; } -run_test("hash_interactions", ({override}) => { +run_test("hash_interactions", ({override, override_rewire}) => { $window_stub = $.create("window-stub"); user_settings.default_view = "recent_topics"; - const helper = test_helper({override, change_tab: true}); + const helper = test_helper({override, override_rewire, change_tab: true}); let recent_view_ui_shown = false; override(recent_view_ui, "show", () => { @@ -331,13 +331,13 @@ run_test("hash_interactions", ({override}) => { helper.assert_events([[ui_util, "blur_active_element"]]); }); -run_test("save_narrow", ({override}) => { - const helper = test_helper({override}); +run_test("save_narrow", ({override, override_rewire}) => { + const helper = test_helper({override, override_rewire}); let operators = [{operator: "is", operand: "dm"}]; blueslip.expect("error", "browser does not support pushState"); - hashchange.save_narrow(operators); + narrow.save_narrow(operators); helper.assert_events([[message_viewport, "stop_auto_scrolling"]]); assert.equal(window.location.hash, "#narrow/is/dm"); @@ -350,7 +350,7 @@ run_test("save_narrow", ({override}) => { operators = [{operator: "is", operand: "starred"}]; helper.clear_events(); - hashchange.save_narrow(operators); + narrow.save_narrow(operators); helper.assert_events([[message_viewport, "stop_auto_scrolling"]]); assert.equal(url_pushed, "http://zulip.zulipdev.com/#narrow/is/starred"); }); diff --git a/web/tests/narrow_activate.test.js b/web/tests/narrow_activate.test.js index dae13d1dc9..f5a3c2ec13 100644 --- a/web/tests/narrow_activate.test.js +++ b/web/tests/narrow_activate.test.js @@ -11,11 +11,13 @@ mock_esm("../src/resize", { }); const all_messages_data = mock_esm("../src/all_messages_data"); +const browser_history = mock_esm("../src/browser_history", { + state: {changing_hash: false}, +}); const compose_actions = mock_esm("../src/compose_actions"); const compose_banner = mock_esm("../src/compose_banner"); const compose_closed_ui = mock_esm("../src/compose_closed_ui"); const compose_recipient = mock_esm("../src/compose_recipient"); -const hashchange = mock_esm("../src/hashchange"); const message_fetch = mock_esm("../src/message_fetch"); const message_list = mock_esm("../src/message_list"); const message_lists = mock_esm("../src/message_lists", { @@ -28,6 +30,7 @@ const message_lists = mock_esm("../src/message_lists", { const message_feed_top_notices = mock_esm("../src/message_feed_top_notices"); const message_feed_loading = mock_esm("../src/message_feed_loading"); const message_view_header = mock_esm("../src/message_view_header"); +const message_viewport = mock_esm("../src/message_viewport"); const narrow_history = mock_esm("../src/narrow_history"); const narrow_title = mock_esm("../src/narrow_title"); const stream_list = mock_esm("../src/stream_list"); @@ -76,16 +79,17 @@ function test_helper({override}) { }); } + stub(browser_history, "set_hash"); stub(compose_banner, "clear_message_sent_banners"); stub(compose_actions, "on_narrow"); stub(compose_closed_ui, "update_reply_recipient_label"); stub(narrow_history, "save_narrow_state_and_flush"); - stub(hashchange, "save_narrow"); stub(message_feed_loading, "hide_indicators"); stub(message_feed_top_notices, "hide_top_of_narrow_notices"); stub(narrow_title, "update_narrow_title"); stub(stream_list, "handle_narrow_activated"); stub(message_view_header, "render_title_area"); + stub(message_viewport, "stop_auto_scrolling"); stub(left_sidebar_navigation_area, "handle_narrow_activated"); stub(typing_events, "render_notifications_for_narrow"); stub(compose_recipient, "update_narrow_to_recipient_visibility"); @@ -185,7 +189,8 @@ run_test("basics", ({override}) => { [compose_banner, "clear_message_sent_banners"], [unread_ops, "process_visible"], [narrow_history, "save_narrow_state_and_flush"], - [hashchange, "save_narrow"], + [message_viewport, "stop_auto_scrolling"], + [browser_history, "set_hash"], [typing_events, "render_notifications_for_narrow"], [compose_closed_ui, "update_buttons_for_stream"], [compose_closed_ui, "update_reply_recipient_label"],