compose_banner: Restrict banner to only JQuery, never htmlString.

Co-authored-by: Anders Kaseorg <anders@zulip.com>

web/src/compose.js

@@ -192,11 +192,14 @@ export function send_message(request = create_message_object()) {
             if (server_error_code === "TOPIC_WILDCARD_MENTION_NOT_ALLOWED") {
                 // The topic wildcard mention permission code path has
                 // a special error.
-                const new_row = render_wildcard_mention_not_allowed_error({
+                const new_row_html = render_wildcard_mention_not_allowed_error({
                     banner_type: compose_banner.ERROR,
                     classname: compose_banner.CLASSNAMES.wildcards_not_allowed,
                 });
-                compose_banner.append_compose_banner_to_banner_list(new_row, $("#compose_banners"));
+                compose_banner.append_compose_banner_to_banner_list(
+                    $(new_row_html),
+                    $("#compose_banners"),
+                );
             } else {
                 compose_banner.show_error_message(
                     response,
@@ -386,12 +389,12 @@ function schedule_message_to_custom_date() {
     const success = function (data) {
         drafts.draft_model.deleteDraft($("textarea#compose-textarea").data("draft-id"));
         clear_compose_box();
-        const new_row = render_success_message_scheduled_banner({
+        const new_row_html = render_success_message_scheduled_banner({
             scheduled_message_id: data.scheduled_message_id,
             deliver_at,
         });
         compose_banner.clear_message_sent_banners();
-        compose_banner.append_compose_banner_to_banner_list(new_row, $banner_container);
+        compose_banner.append_compose_banner_to_banner_list($(new_row_html), $banner_container);
     };
 
     const error = function (xhr) {

web/src/compose_banner.ts

@@ -66,22 +66,22 @@ export function get_compose_banner_container($textarea: JQuery): JQuery {
 // to a banner container. The function accepts a container element
 // as a parameter, to which a banner should be appended.
 export function append_compose_banner_to_banner_list(
-    banner: JQuery | JQuery.htmlString,
+    $banner: JQuery,
     $list_container: JQuery,
 ): void {
-    scroll_util.get_content_element($list_container).append(banner);
+    scroll_util.get_content_element($list_container).append($banner);
 }
 
 export function update_or_append_banner(
-    banner: JQuery | JQuery.htmlString,
+    $banner: JQuery,
     banner_classname: string,
     $list_container: JQuery,
 ): void {
-    const $banner = $list_container.find(`.${CSS.escape(banner_classname)}`);
+    const $existing_banner = $list_container.find(`.${CSS.escape(banner_classname)}`);
-    if ($banner.length === 0) {
+    if ($existing_banner.length === 0) {
-        append_compose_banner_to_banner_list(banner, $list_container);
+        append_compose_banner_to_banner_list($banner, $list_container);
     } else {
-        $banner.replaceWith(banner);
+        $existing_banner.replaceWith($banner);
     }
 }
 
@@ -147,7 +147,7 @@ export function show_error_message(
     // we remove the banner with a matched classname.
     $container.find(`.${CSS.escape(classname)}`).remove();
 
-    const new_row = render_compose_banner({
+    const new_row_html = render_compose_banner({
         banner_type: ERROR,
         stream_id: null,
         topic_name: null,
@@ -155,7 +155,7 @@ export function show_error_message(
         button_text: null,
         classname,
     });
-    append_compose_banner_to_banner_list(new_row, $container);
+    append_compose_banner_to_banner_list($(new_row_html), $container);
 
     hide_compose_spinner();
 
@@ -168,12 +168,12 @@ export function show_stream_does_not_exist_error(stream_name: string): void {
     // Remove any existing banners with this warning.
     $(`#compose_banners .${CSS.escape(CLASSNAMES.stream_does_not_exist)}`).remove();
 
-    const new_row = render_stream_does_not_exist_error({
+    const new_row_html = render_stream_does_not_exist_error({
         banner_type: ERROR,
         stream_name,
         classname: CLASSNAMES.stream_does_not_exist,
     });
-    append_compose_banner_to_banner_list(new_row, $("#compose_banners"));
+    append_compose_banner_to_banner_list($(new_row_html), $("#compose_banners"));
     hide_compose_spinner();
 
     // Open stream select dropdown.

web/src/compose_validate.js

@@ -163,13 +163,13 @@ export function warn_if_private_stream_is_linked(linked_stream, $textarea) {
         return;
     }
 
-    const new_row = render_private_stream_warning({
+    const new_row_html = render_private_stream_warning({
         banner_type: compose_banner.WARNING,
         stream_name: linked_stream.name,
         classname: compose_banner.CLASSNAMES.private_stream_warning,
     });
     const $container = compose_banner.get_compose_banner_container($textarea);
-    compose_banner.append_compose_banner_to_banner_list(new_row, $container);
+    compose_banner.append_compose_banner_to_banner_list($(new_row_html), $container);
 }
 
 export function warn_if_mentioning_unsubscribed_user(mentioned, $textarea) {
@@ -216,9 +216,9 @@ export function warn_if_mentioning_unsubscribed_user(mentioned, $textarea) {
                 should_add_guest_user_indicator: people.should_add_guest_user_indicator(user_id),
             };
 
-            const new_row = render_not_subscribed_warning(context);
+            const new_row_html = render_not_subscribed_warning(context);
             const $container = compose_banner.get_compose_banner_container($textarea);
-            compose_banner.append_compose_banner_to_banner_list(new_row, $container);
+            compose_banner.append_compose_banner_to_banner_list($(new_row_html), $container);
         }
     }
 }
@@ -277,8 +277,8 @@ export function warn_if_topic_resolved(topic_changed) {
             classname: compose_banner.CLASSNAMES.topic_resolved,
         };
 
-        const new_row = render_compose_banner(context);
+        const new_row_html = render_compose_banner(context);
-        compose_banner.append_compose_banner_to_banner_list(new_row, $("#compose_banners"));
+        compose_banner.append_compose_banner_to_banner_list($(new_row_html), $("#compose_banners"));
         compose_state.set_recipient_viewed_topic_resolved_banner(true);
     } else {
         clear_topic_resolved_warning();
@@ -297,8 +297,8 @@ export function warn_if_in_search_view() {
             classname: compose_banner.CLASSNAMES.search_view,
         };
 
-        const new_row = render_compose_banner(context);
+        const new_row_html = render_compose_banner(context);
-        compose_banner.append_compose_banner_to_banner_list(new_row, $("#compose_banners"));
+        compose_banner.append_compose_banner_to_banner_list($(new_row_html), $("#compose_banners"));
     }
 }
 
@@ -316,7 +316,7 @@ function show_stream_wildcard_warnings(opts) {
         button_text = $t({defaultMessage: "Yes, save"});
     }
 
-    const stream_wildcard_template = render_stream_wildcard_warning({
+    const stream_wildcard_html = render_stream_wildcard_warning({
         banner_type: compose_banner.WARNING,
         subscriber_count,
         stream_name,
@@ -330,13 +330,13 @@ function show_stream_wildcard_warnings(opts) {
     // only show one error for any number of @all or @everyone mentions
     if (opts.$banner_container.find(`.${CSS.escape(classname)}`).length === 0) {
         compose_banner.append_compose_banner_to_banner_list(
-            stream_wildcard_template,
+            $(stream_wildcard_html),
             opts.$banner_container,
         );
     } else {
         // if there is already a banner, replace it with the new one
         compose_banner.update_or_append_banner(
-            stream_wildcard_template,
+            $(stream_wildcard_html),
             classname,
             opts.$banner_container,
         );
@@ -517,12 +517,15 @@ export function validate_stream_message_mentions(opts) {
     // if they haven't acknowledged the wildcard warning yet.
     if (opts.stream_wildcard_mention !== null && subscriber_count > wildcard_mention_threshold) {
         if (!wildcard_mention_policy_authorizes_user()) {
-            const new_row = render_wildcard_mention_not_allowed_error({
+            const new_row_html = render_wildcard_mention_not_allowed_error({
                 banner_type: compose_banner.ERROR,
                 classname: compose_banner.CLASSNAMES.wildcards_not_allowed,
                 stream_wildcard_mention: opts.stream_wildcard_mention,
             });
-            compose_banner.append_compose_banner_to_banner_list(new_row, opts.$banner_container);
+            compose_banner.append_compose_banner_to_banner_list(
+                $(new_row_html),
+                opts.$banner_container,
+            );
             return false;
         }
 
@@ -565,7 +568,7 @@ export function validation_error(error_type, stream_name) {
                 return false;
             }
             const sub = stream_data.get_sub(stream_name);
-            const new_row = render_compose_banner({
+            const new_row_html = render_compose_banner({
                 banner_type: compose_banner.ERROR,
                 banner_text: $t({
                     defaultMessage:
@@ -579,7 +582,7 @@ export function validation_error(error_type, stream_name) {
                 // closing the banner would be more confusing than helpful.
                 hide_close_button: true,
             });
-            compose_banner.append_compose_banner_to_banner_list(new_row, $banner_container);
+            compose_banner.append_compose_banner_to_banner_list($(new_row_html), $banner_container);
             return false;
         }
     }

web/src/message_edit.js

@@ -1083,11 +1083,14 @@ export function save_message_row_edit($row) {
                     );
 
                     if (xhr.responseJSON?.code === "TOPIC_WILDCARD_MENTION_NOT_ALLOWED") {
-                        const new_row = render_wildcard_mention_not_allowed_error({
+                        const new_row_html = render_wildcard_mention_not_allowed_error({
                             banner_type: compose_banner.ERROR,
                             classname: compose_banner.CLASSNAMES.wildcards_not_allowed,
                         });
-                        compose_banner.append_compose_banner_to_banner_list(new_row, $container);
+                        compose_banner.append_compose_banner_to_banner_list(
+                            $(new_row_html),
+                            $container,
+                        );
                         return;
                     }
 

web/src/scheduled_messages_ui.js

@@ -73,7 +73,7 @@ function show_message_unscheduled_banner(scheduled_delivery_timestamp) {
         new Date(scheduled_delivery_timestamp * 1000),
         "time",
     );
-    const unscheduled_banner = render_compose_banner({
+    const unscheduled_banner_html = render_compose_banner({
         banner_type: compose_banner.WARNING,
         banner_text: $t({
             defaultMessage: "This message is no longer scheduled to be sent.",
@@ -81,7 +81,10 @@ function show_message_unscheduled_banner(scheduled_delivery_timestamp) {
         button_text: $t({defaultMessage: "Schedule for {deliver_at}"}, {deliver_at}),
         classname: compose_banner.CLASSNAMES.unscheduled_message,
     });
-    compose_banner.append_compose_banner_to_banner_list(unscheduled_banner, $("#compose_banners"));
+    compose_banner.append_compose_banner_to_banner_list(
+        $(unscheduled_banner_html),
+        $("#compose_banners"),
+    );
 }
 
 export function edit_scheduled_message(scheduled_message_id, should_narrow_to_recipient = true) {

web/src/upload.js

@@ -146,14 +146,14 @@ function add_upload_banner(
     file_id,
     is_upload_process_tracker = false,
 ) {
-    const new_banner = render_upload_banner({
+    const new_banner_html = render_upload_banner({
         banner_type,
         is_upload_process_tracker,
         banner_text,
         file_id,
     });
     compose_banner.append_compose_banner_to_banner_list(
-        new_banner,
+        $(new_banner_html),
         get_item("banner_container", config),
     );
 }

web/tests/compose.test.js

@@ -327,6 +327,7 @@ test_ui("send_message", ({override, override_rewire, mock_template}) => {
             assert.equal(data.classname, "generic_compose_error");
             assert.equal(data.banner_text, "Error sending message: Server says 408");
             banner_rendered = true;
+            return "<banner-stub>";
         });
         stub_state = initialize_state_stub_dict();
         $("textarea#compose-textarea").val("foobarfoobar");

web/tests/compose_validate.test.js

@@ -130,6 +130,7 @@ test_ui("validate_stream_message_address_info", ({mock_template}) => {
         assert.equal(data.classname, compose_banner.CLASSNAMES.stream_does_not_exist);
         assert.equal(data.stream_name, "Frontend");
         stream_does_not_exist_rendered = true;
+        return "<banner-stub>";
     });
     channel.post = (payload) => {
         assert.equal(payload.data.stream, "Frontend");
@@ -143,6 +144,7 @@ test_ui("validate_stream_message_address_info", ({mock_template}) => {
         assert.equal(data.classname, "subscription_error");
         assert.equal(data.banner_text, $t({defaultMessage: "Error checking subscription."}));
         subscription_error_rendered = true;
+        return "<banner-stub>";
     });
     channel.post = (payload) => {
         assert.equal(payload.data.stream, "social");
@@ -195,6 +197,7 @@ test_ui("validate", ({mock_template}) => {
             $t({defaultMessage: "Please specify at least one valid recipient."}),
         );
         pm_recipient_error_rendered = true;
+        return "<banner-stub>";
     });
     assert.ok(!compose_validate.validate());
     assert.ok(pm_recipient_error_rendered);
@@ -215,6 +218,7 @@ test_ui("validate", ({mock_template}) => {
             $t({defaultMessage: "You cannot send messages to deactivated users."}),
         );
         deactivated_user_error_rendered = true;
+        return "<banner-stub>";
     });
     assert.ok(!compose_validate.validate());
     assert.ok(deactivated_user_error_rendered);
@@ -240,6 +244,7 @@ test_ui("validate", ({mock_template}) => {
             );
             zephyr_error_rendered = true;
         }
+        return "<banner-stub>";
     });
     initialize_pm_pill();
     compose_state.private_message_recipient("welcome-bot@example.com");
@@ -274,6 +279,7 @@ test_ui("validate", ({mock_template}) => {
         assert.equal(data.classname, compose_banner.CLASSNAMES.missing_stream);
         assert.equal(data.banner_text, $t({defaultMessage: "Please specify a stream."}));
         empty_stream_error_rendered = true;
+        return "<banner-stub>";
     });
     assert.ok(!compose_validate.validate());
     assert.ok(empty_stream_error_rendered);
@@ -294,6 +300,7 @@ test_ui("validate", ({mock_template}) => {
             $t({defaultMessage: "Topics are required in this organization."}),
         );
         missing_topic_error_rendered = true;
+        return "<banner-stub>";
     });
     assert.ok(!compose_validate.validate());
     assert.ok(missing_topic_error_rendered);
@@ -418,6 +425,7 @@ test_ui("validate_stream_message", ({override_rewire, mock_template}) => {
     mock_template("compose_banner/stream_wildcard_warning.hbs", false, (data) => {
         stream_wildcard_warning_rendered = true;
         assert.equal(data.subscriber_count, 16);
+        return "<banner-stub>";
     });
 
     override_rewire(compose_validate, "wildcard_mention_policy_authorizes_user", () => true);
@@ -431,6 +439,7 @@ test_ui("validate_stream_message", ({override_rewire, mock_template}) => {
         assert.equal(data.classname, compose_banner.CLASSNAMES.wildcards_not_allowed);
         assert.equal(data.stream_wildcard_mention, "all");
         wildcards_not_allowed_rendered = true;
+        return "<banner-stub>";
     });
     override_rewire(compose_validate, "wildcard_mention_policy_authorizes_user", () => false);
     assert.ok(!compose_validate.validate());
@@ -465,6 +474,7 @@ test_ui("test_validate_stream_message_post_policy_admin_only", ({mock_template})
             }),
         );
         banner_rendered = true;
+        return "<banner-stub>";
     });
     assert.ok(!compose_validate.validate());
     assert.ok(banner_rendered);
@@ -509,6 +519,7 @@ test_ui("test_validate_stream_message_post_policy_moderators_only", ({mock_templ
             }),
         );
         banner_rendered = true;
+        return "<banner-stub>";
     });
     assert.ok(!compose_validate.validate());
     assert.ok(banner_rendered);
@@ -544,6 +555,7 @@ test_ui("test_validate_stream_message_post_policy_full_members_only", ({mock_tem
             }),
         );
         banner_rendered = true;
+        return "<banner-stub>";
     });
     assert.ok(!compose_validate.validate());
     assert.ok(banner_rendered);
@@ -565,6 +577,7 @@ test_ui("test_check_overflow_text", ({mock_template}) => {
             }),
         );
         banner_rendered = true;
+        return "<banner-stub>";
     });
 
     // Indicator should show red colored text
@@ -658,7 +671,7 @@ test_ui("warn_if_private_stream_is_linked", ({mock_template}) => {
         assert.equal(data.classname, compose_banner.CLASSNAMES.private_stream_warning);
         assert.equal(data.stream_name, "Denmark");
         banner_rendered = true;
-        return "private_stream_warning_stub";
+        return "<banner-stub>";
     });
 
     function test_noop_case(invite_only) {
@@ -708,6 +721,7 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => {
         assert.equal(data.stream_id, 111);
         assert.equal(data.name, "Foo Barson");
         new_banner_rendered = true;
+        return "<banner-stub>";
     });
 
     function test_noop_case(is_private, is_zephyr_mirror, is_broadcast) {
@@ -795,7 +809,7 @@ test_ui("test warn_if_topic_resolved", ({override, mock_template}) => {
             }),
         );
         error_shown = true;
-        return "topic_resolved_warning_stub";
+        return "<banner-stub>";
     });
 
     const sub = {

web/tests/upload.test.js

@@ -189,6 +189,7 @@ test("show_error_message", ({mock_template}) => {
         assert.equal(data.banner_type, "error");
         assert.equal(data.banner_text, "Error message");
         banner_shown = true;
+        return "<banner-stub>";
     });
 
     $("#compose-send-button").prop("disabled", true);
@@ -201,6 +202,7 @@ test("show_error_message", ({mock_template}) => {
         assert.equal(data.banner_type, "error");
         assert.equal(data.banner_text, "translated: An unknown error occurred.");
         banner_shown = true;
+        return "<banner-stub>";
     });
     upload.show_error_message({mode: "compose"});
 });
@@ -249,6 +251,7 @@ test("upload_files", async ({mock_template, override_rewire}) => {
             "translated: File and image uploads have been disabled for this organization.",
         );
         banner_shown = true;
+        return "<banner-stub>";
     });
     page_params.max_file_upload_size_mib = 0;
     $("#compose_banners .upload_banner .upload_msg").text("");
@@ -284,6 +287,7 @@ test("upload_files", async ({mock_template, override_rewire}) => {
     banner_shown = false;
     mock_template("compose_banner/upload_banner.hbs", false, () => {
         banner_shown = true;
+        return "<banner-stub>";
     });
     await upload.upload_files(uppy, config, files);
     assert.ok($(".message-send-controls").hasClass("disabled-message-send-controls"));
@@ -575,6 +579,7 @@ test("uppy_events", ({override_rewire, mock_template}) => {
     mock_template("compose_banner/upload_banner.hbs", false, (data) => {
         assert.equal(data.banner_type, "error");
         assert.equal(data.banner_text, "Some error message");
+        return "<banner-stub>";
     });
     state = {
         type: "error",