Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

popover_menus: Fix event handler trying to hide a hidden popover. 

When user is trying to open a modal after clicking on a button
in a popover, we call popovers.hide_all() before opening the modal
which hides the popover but since the event handler call isn't
finished running yet, we call instance.hide() again resulting in
tippy throwing errors that this could be a memory leak.

We introduce a wrapper function for `instance.hide` which if
the popover/tooltip is visible before hiding it to fix it.
This commit is contained in:
Aman Agrawal 2024-04-02 07:29:56 +00:00 committed by Tim Abbott
parent ac5161f439
commit 0a90a13bec
12 changed files with 62 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
web/src/click_handlers.js
View File

@ -25,6 +25,7 @@ import * as narrow_state from "./narrow_state";
import * as navigate from "./navigate"; import * as navigate from "./navigate";
import {page_params} from "./page_params"; import {page_params} from "./page_params";
import * as pm_list from "./pm_list"; import * as pm_list from "./pm_list";
import * as popover_menus from "./popover_menus";
import * as reactions from "./reactions"; import * as reactions from "./reactions";
import * as recent_view_ui from "./recent_view_ui"; import * as recent_view_ui from "./recent_view_ui";
import * as rows from "./rows"; import * as rows from "./rows";
@ -542,7 +543,7 @@ export function initialize() {
for (const mutation of mutationsList) { for (const mutation of mutationsList) {
// Hide instance if reference is in the removed node list. // Hide instance if reference is in the removed node list.
if (check_reference_removed(mutation, instance)) { if (check_reference_removed(mutation, instance)) {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
} }
} }
}; };

4
web/src/compose_popovers.js
View File

@ -41,12 +41,12 @@ export function initialize() {
$popper.one("click", ".compose_mobile_stream_button", (e) => { $popper.one("click", ".compose_mobile_stream_button", (e) => {
compose_actions.start("stream", {trigger: "clear topic button"}); compose_actions.start("stream", {trigger: "clear topic button"});
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".compose_mobile_direct_message_button", (e) => { $popper.one("click", ".compose_mobile_direct_message_button", (e) => {
compose_actions.start("private"); compose_actions.start("private");
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
}, },
onHidden(instance) { onHidden(instance) {

6
web/src/dropdown_widget.ts
View File

@ -12,7 +12,7 @@ import * as blueslip from "./blueslip";
import * as ListWidget from "./list_widget"; import * as ListWidget from "./list_widget";
import type {ListWidget as ListWidgetType} from "./list_widget"; import type {ListWidget as ListWidgetType} from "./list_widget";
import {page_params} from "./page_params"; import {page_params} from "./page_params";
import {default_popover_props} from "./popover_menus"; import * as popover_menus from "./popover_menus";
import type {StreamSubscription} from "./sub_store"; import type {StreamSubscription} from "./sub_store";
import {parse_html} from "./ui_util"; import {parse_html} from "./ui_util";
@ -180,7 +180,7 @@ export class DropdownWidget {
return; return;
} }
this.instance = tippy.delegate(delegate_container, { this.instance = tippy.delegate(delegate_container, {
...default_popover_props, ...popover_menus.default_popover_props,
target: this.widget_selector, target: this.widget_selector,
// Custom theme defined in popovers.css // Custom theme defined in popovers.css
theme: "dropdown-widget", theme: "dropdown-widget",
@ -286,7 +286,7 @@ export class DropdownWidget {
break; break;
case "Escape": case "Escape":
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
this.on_exit_with_escape_callback(); this.on_exit_with_escape_callback();
e.stopPropagation(); e.stopPropagation();
e.preventDefault(); e.preventDefault();

8
web/src/gear_menu.js
View File

@ -145,13 +145,13 @@ export function initialize() {
}); });
}, },
); );
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
}); });
$popper.on("click", ".change-language-spectator", (e) => { $popper.on("click", ".change-language-spectator", (e) => {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
settings_preferences.launch_default_language_setting_modal(); settings_preferences.launch_default_language_setting_modal();
@ -162,7 +162,7 @@ export function initialize() {
// Also, since these buttons are only visible for spectators which doesn't have events, // Also, since these buttons are only visible for spectators which doesn't have events,
// if theme is changed in a different tab, the theme of this tab remains the same. // if theme is changed in a different tab, the theme of this tab remains the same.
$popper.on("click", "#gear-menu-dropdown .gear-menu-select-dark-theme", (e) => { $popper.on("click", "#gear-menu-dropdown .gear-menu-select-dark-theme", (e) => {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
requestAnimationFrame(() => { requestAnimationFrame(() => {
@ -172,7 +172,7 @@ export function initialize() {
}); });
$popper.on("click", "#gear-menu-dropdown .gear-menu-select-light-theme", (e) => { $popper.on("click", "#gear-menu-dropdown .gear-menu-select-light-theme", (e) => {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
requestAnimationFrame(() => { requestAnimationFrame(() => {

8
web/src/left_sidebar_navigation_area_popovers.js
View File

@ -38,7 +38,7 @@ function common_click_handlers() {
function register_mark_all_read_handler(event) { function register_mark_all_read_handler(event) {
const {instance} = event.data; const {instance} = event.data;
unread_ops.confirm_mark_all_as_read(); unread_ops.confirm_mark_all_as_read();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
} }
export function initialize() { export function initialize() {
@ -52,7 +52,7 @@ export function initialize() {
$popper.one("click", "#unstar_all_messages", () => { $popper.one("click", "#unstar_all_messages", () => {
starred_messages_ui.confirm_unstar_all_messages(); starred_messages_ui.confirm_unstar_all_messages();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", "#toggle_display_starred_msg_count", () => { $popper.one("click", "#toggle_display_starred_msg_count", () => {
const data = {}; const data = {};
@ -63,7 +63,7 @@ export function initialize() {
url: "/json/settings", url: "/json/settings",
data, data,
}); });
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
}, },
onShow(instance) { onShow(instance) {
@ -97,7 +97,7 @@ export function initialize() {
$popper.one("click", "#delete_all_drafts_sidebar", () => { $popper.one("click", "#delete_all_drafts_sidebar", () => {
drafts.confirm_delete_all_drafts(); drafts.confirm_delete_all_drafts();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
}, },
onShow(instance) { onShow(instance) {

20
web/src/message_actions_popover.js
View File

@ -123,7 +123,7 @@ export function initialize() {
}); });
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".popover_edit_message, .popover_view_source", (e) => { $popper.one("click", ".popover_edit_message, .popover_view_source", (e) => {
@ -133,7 +133,7 @@ export function initialize() {
message_edit.start($row); message_edit.start($row);
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".popover_move_message", (e) => { $popper.one("click", ".popover_move_message", (e) => {
@ -149,7 +149,7 @@ export function initialize() {
); );
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".mark_as_unread", (e) => { $popper.one("click", ".mark_as_unread", (e) => {
@ -157,7 +157,7 @@ export function initialize() {
unread_ops.mark_as_unread_from_here(message_id); unread_ops.mark_as_unread_from_here(message_id);
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".popover_toggle_collapse", (e) => { $popper.one("click", ".popover_toggle_collapse", (e) => {
@ -172,7 +172,7 @@ export function initialize() {
} }
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".rehide_muted_user_message", (e) => { $popper.one("click", ".rehide_muted_user_message", (e) => {
@ -188,7 +188,7 @@ export function initialize() {
} }
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".view_read_receipts", (e) => { $popper.one("click", ".view_read_receipts", (e) => {
@ -196,7 +196,7 @@ export function initialize() {
read_receipts.show_user_list(message_id); read_receipts.show_user_list(message_id);
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".delete_message", (e) => { $popper.one("click", ".delete_message", (e) => {
@ -204,7 +204,7 @@ export function initialize() {
message_edit.delete_message(message_id); message_edit.delete_message(message_id);
e.preventDefault(); e.preventDefault();
e.stopPropagation(); e.stopPropagation();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".reaction_button", (e) => { $popper.one("click", ".reaction_button", (e) => {
@ -216,7 +216,7 @@ export function initialize() {
emoji_picker.toggle_emoji_popover(instance.reference.parentElement, message_id, { emoji_picker.toggle_emoji_popover(instance.reference.parentElement, message_id, {
placement: "bottom", placement: "bottom",
}); });
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
new ClipboardJS($popper.find(".copy_link")[0]).on("success", () => { new ClipboardJS($popper.find(".copy_link")[0]).on("success", () => {
@ -226,7 +226,7 @@ export function initialize() {
// We unfocus this so keyboard shortcuts, etc., will work again. // We unfocus this so keyboard shortcuts, etc., will work again.
$(":focus").trigger("blur"); $(":focus").trigger("blur");
}, 0); }, 0);
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
}, },
onHidden(instance) { onHidden(instance) {

7
web/src/message_list_tooltips.ts
View File

@ -10,6 +10,7 @@ import render_narrow_tooltip from "../templates/narrow_tooltip.hbs";
import {$t} from "./i18n"; import {$t} from "./i18n";
import * as message_lists from "./message_lists"; import * as message_lists from "./message_lists";
import * as popover_menus from "./popover_menus";
import * as reactions from "./reactions"; import * as reactions from "./reactions";
import * as rows from "./rows"; import * as rows from "./rows";
import {realm} from "./state_data"; import {realm} from "./state_data";
@ -83,7 +84,7 @@ function hide_tooltip_if_reference_removed(
// We have to be smart about hiding the instance, so we hide it as soon // We have to be smart about hiding the instance, so we hide it as soon
// as it is displayed. // as it is displayed.
setTimeout(() => { setTimeout(() => {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}, 0); }, 0);
return; return;
} }
@ -92,11 +93,11 @@ function hide_tooltip_if_reference_removed(
for (const node of nodes_to_check_for_removal) { for (const node of nodes_to_check_for_removal) {
// Hide instance if reference's class changes. // Hide instance if reference's class changes.
if (mutation.type === "attributes" && mutation.attributeName === "class") { if (mutation.type === "attributes" && mutation.attributeName === "class") {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
} }
// Hide instance if reference is in the removed node list. // Hide instance if reference is in the removed node list.
if (Array.prototype.includes.call(mutation.removedNodes, node)) { if (Array.prototype.includes.call(mutation.removedNodes, node)) {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
} }
} }
} }

2
web/src/personal_menu_popover.js
View File

@ -71,7 +71,7 @@ export function initialize() {
emoji_name: "", emoji_name: "",
emoji_code: "", emoji_code: "",
success() { success() {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}, },
}); });
}); });

19
web/src/popover_menus.ts
View File

@ -149,6 +149,15 @@ function get_popover_items_for_instance(instance: PopoverInstance): JQuery | und
return $current_elem.find("a, [tabindex='0']").filter(":visible"); return $current_elem.find("a, [tabindex='0']").filter(":visible");
} }
export function hide_current_popover_if_visible(instance: PopoverInstance | null): void {
// Call this function instead of `instance.hide` to avoid tippy
// logging about the possibility of already hidden instances,
// which can occur when a click handler does a hide_all().
if (instance?.state.isVisible) {
instance.hide();
}
}
export const default_popover_props: Partial<PopoverProps> = { export const default_popover_props: Partial<PopoverProps> = {
delay: 0, delay: 0,
appendTo: () => document.body, appendTo: () => document.body,
@ -191,7 +200,7 @@ export const default_popover_props: Partial<PopoverProps> = {
"referenceHidden", "referenceHidden",
); );
if (is_reference_outside_window) { if (is_reference_outside_window) {
instance.hide(); hide_current_popover_if_visible(instance);
return; return;
} }
@ -250,7 +259,7 @@ export const default_popover_props: Partial<PopoverProps> = {
element.classList.contains("sticky_header"), element.classList.contains("sticky_header"),
) )
) { ) {
instance.hide(); hide_current_popover_if_visible(instance);
} }
}, },
}, },
@ -283,7 +292,7 @@ export function on_show_prep(instance: PopoverInstance): void {
$(instance.popper).one("click", ".navigate_and_close_popover", (e) => { $(instance.popper).one("click", ".navigate_and_close_popover", (e) => {
// Handler for links inside popover which don't need a special click handler. // Handler for links inside popover which don't need a special click handler.
e.stopPropagation(); e.stopPropagation();
instance.hide(); hide_current_popover_if_visible(instance);
}); });
} }
@ -361,7 +370,7 @@ export function toggle_popover_menu(
): PopoverInstance { ): PopoverInstance {
const instance = target._tippy; const instance = target._tippy;
if (instance) { if (instance) {
instance.hide(); hide_current_popover_if_visible(instance);
return instance; return instance;
} }
@ -415,7 +424,7 @@ export function register_popover_menu(target: string, popover_props: Partial<Pop
const instance = toggle_popover_menu(e.currentTarget, popover_props); const instance = toggle_popover_menu(e.currentTarget, popover_props);
const $popper = $(instance.popper); const $popper = $(instance.popper);
$popper.on("click", "a[href]", () => { $popper.on("click", "a[href]", () => {
instance.hide(); hide_current_popover_if_visible(instance);
}); });
}); });
} }

6
web/src/scheduled_messages_popover.js
View File

@ -173,7 +173,7 @@ export function initialize() {
$popper.one("click", ".send_later_selected_send_later_time", () => { $popper.one("click", ".send_later_selected_send_later_time", () => {
const send_at_timestamp = scheduled_messages.get_selected_send_later_timestamp(); const send_at_timestamp = scheduled_messages.get_selected_send_later_timestamp();
do_schedule_message(send_at_timestamp); do_schedule_message(send_at_timestamp);
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
// Handle clicks on Enter-to-send settings // Handle clicks on Enter-to-send settings
$popper.one("click", ".enter_sends_choice", (e) => { $popper.one("click", ".enter_sends_choice", (e) => {
@ -195,13 +195,13 @@ export function initialize() {
}); });
e.stopPropagation(); e.stopPropagation();
setTimeout(() => { setTimeout(() => {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}, ENTER_SENDS_SELECTION_DELAY); }, ENTER_SENDS_SELECTION_DELAY);
}); });
// Handle Send later clicks // Handle Send later clicks
$popper.one("click", ".open_send_later_modal", () => { $popper.one("click", ".open_send_later_modal", () => {
open_send_later_menu(); open_send_later_menu();
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
}, },
onHidden(instance) { onHidden(instance) {

26
web/src/topic_popover.js
View File

@ -58,7 +58,7 @@ export function initialize() {
const {stream_id, topic_name} = instance.context; const {stream_id, topic_name} = instance.context;
if (!stream_id) { if (!stream_id) {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
return; return;
} }
@ -80,7 +80,7 @@ export function initialize() {
topic_name, topic_name,
user_topics.all_visibility_policies.UNMUTED, user_topics.all_visibility_policies.UNMUTED,
); );
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-remove-unmute", () => { $popper.one("click", ".sidebar-popover-remove-unmute", () => {
@ -89,7 +89,7 @@ export function initialize() {
topic_name, topic_name,
user_topics.all_visibility_policies.INHERIT, user_topics.all_visibility_policies.INHERIT,
); );
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-mute-topic", () => { $popper.one("click", ".sidebar-popover-mute-topic", () => {
@ -98,7 +98,7 @@ export function initialize() {
topic_name, topic_name,
user_topics.all_visibility_policies.MUTED, user_topics.all_visibility_policies.MUTED,
); );
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-remove-mute", () => { $popper.one("click", ".sidebar-popover-remove-mute", () => {
@ -107,7 +107,7 @@ export function initialize() {
topic_name, topic_name,
user_topics.all_visibility_policies.INHERIT, user_topics.all_visibility_policies.INHERIT,
); );
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-unstar-all-in-topic", () => { $popper.one("click", ".sidebar-popover-unstar-all-in-topic", () => {
@ -115,17 +115,17 @@ export function initialize() {
Number.parseInt(stream_id, 10), Number.parseInt(stream_id, 10),
topic_name, topic_name,
); );
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-mark-topic-read", () => { $popper.one("click", ".sidebar-popover-mark-topic-read", () => {
unread_ops.mark_topic_as_read(stream_id, topic_name); unread_ops.mark_topic_as_read(stream_id, topic_name);
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-mark-topic-unread", () => { $popper.one("click", ".sidebar-popover-mark-topic-unread", () => {
unread_ops.mark_topic_as_unread(stream_id, topic_name); unread_ops.mark_topic_as_unread(stream_id, topic_name);
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-delete-topic-messages", () => { $popper.one("click", ".sidebar-popover-delete-topic-messages", () => {
@ -140,7 +140,7 @@ export function initialize() {
}, },
}); });
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-toggle-resolved", () => { $popper.one("click", ".sidebar-popover-toggle-resolved", () => {
@ -148,23 +148,23 @@ export function initialize() {
message_edit.toggle_resolve_topic(message_id, topic_name, true); message_edit.toggle_resolve_topic(message_id, topic_name, true);
}); });
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-move-topic-messages", () => { $popper.one("click", ".sidebar-popover-move-topic-messages", () => {
stream_popover.build_move_topic_to_stream_popover(stream_id, topic_name, false); stream_popover.build_move_topic_to_stream_popover(stream_id, topic_name, false);
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
$popper.one("click", ".sidebar-popover-rename-topic-messages", () => { $popper.one("click", ".sidebar-popover-rename-topic-messages", () => {
stream_popover.build_move_topic_to_stream_popover(stream_id, topic_name, true); stream_popover.build_move_topic_to_stream_popover(stream_id, topic_name, true);
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
new ClipboardJS($popper.find(".sidebar-popover-copy-link-to-topic")[0]).on( new ClipboardJS($popper.find(".sidebar-popover-copy-link-to-topic")[0]).on(
"success", "success",
() => { () => {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}, },
); );
}, },

4
web/src/user_topic_popover.js
View File

@ -43,7 +43,7 @@ export function initialize() {
const {stream_id, topic_name} = instance.context; const {stream_id, topic_name} = instance.context;
if (!stream_id) { if (!stream_id) {
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
return; return;
} }
@ -58,7 +58,7 @@ export function initialize() {
topic_name, topic_name,
visibility_policy, visibility_policy,
); );
instance.hide(); popover_menus.hide_current_popover_if_visible(instance);
}); });
}, },
onHidden(instance) { onHidden(instance) {