widgets: Always render and activate widgets in the current view.

Earlier we did not activate widgets if their rendered HTML was cached,
and also when narrowing to the combined feed view. This caused bugs with
widgets not being activated, and so not responding to any interactions.

Now we unconditionally render and activate widgets in the current view,
irrespective of the cached HTML or the view being narrowed to.

web/src/narrow.js

@@ -49,13 +49,13 @@ import * as spectators from "./spectators";
 import {realm} from "./state_data";
 import * as stream_data from "./stream_data";
 import * as stream_list from "./stream_list";
+import * as submessage from "./submessage";
 import * as topic_generator from "./topic_generator";
 import * as typing_events from "./typing_events";
 import * as unread_ops from "./unread_ops";
 import * as unread_ui from "./unread_ui";
 import {user_settings} from "./user_settings";
 import * as util from "./util";
-import * as widgetize from "./widgetize";
 
 const LARGER_THAN_MAX_MESSAGE_ID = 10000000000000000;
 
@@ -499,7 +499,7 @@ export function activate(raw_terms, opts) {
             // Reset the collapsed status of messages rows.
             condense.condense_and_collapse(message_lists.current.view.$list.find(".message_row"));
             message_edit.restore_edit_state_after_message_view_change();
-            widgetize.set_widgets_for_list();
+            submessage.process_widget_rows_in_list(message_lists.current);
             message_feed_top_notices.update_top_of_narrow_notices(msg_list);
 
             // We may need to scroll to the selected message after swapping

web/src/submessage.ts

@@ -2,6 +2,7 @@ import {z} from "zod";
 
 import * as blueslip from "./blueslip";
 import * as channel from "./channel";
+import type {MessageList} from "./message_lists";
 import * as message_store from "./message_store";
 import type {Message} from "./message_store";
 import type {Submessage} from "./types";
@@ -87,6 +88,14 @@ export function get_message_events(message: Message): SubmessageEvents | undefin
     return clean_events;
 }
 
+export function process_widget_rows_in_list(list: MessageList | undefined): void {
+    for (const message_id of widgetize.widget_contents.keys()) {
+        if (list?.get(message_id) !== undefined) {
+            process_submessages({message_id, $row: list.get_row(message_id)});
+        }
+    }
+}
+
 export function process_submessages(in_opts: {$row: JQuery; message_id: number}): void {
     // This happens in our rendering path, so we try to limit any
     // damage that may be triggered by one rogue message.

web/src/widgetize.ts

@@ -47,15 +47,7 @@ export function clear_for_testing(): void {
 
 function set_widget_in_message($row: JQuery, $widget_elem: JQuery): void {
     const $content_holder = $row.find(".message_content");
-
-    // Avoid adding the $widget_elem if it already exists.
-    // This can happen when the app loads in the "Recent Conversations"
-    // view and the user changes the view to "Combined feed".
-    // This is important since jQuery removes all the event handlers
-    // on `empty()`ing an element.
-    if ($content_holder.find(".widget-content").length === 0) {
     $content_holder.empty().append($widget_elem);
-    }
 }
 
 export function activate(in_opts: WidgetOptions): void {
@@ -86,15 +78,9 @@ export function activate(in_opts: WidgetOptions): void {
         return;
     }
 
-    let $widget_elem = widget_contents.get(message.id);
-    if ($widget_elem) {
-        set_widget_in_message($row, $widget_elem);
-        return;
-    }
-
     // We depend on our widgets to use templates to build
     // the HTML that will eventually go in this div.
-    $widget_elem = $("<div>").addClass("widget-content");
+    const $widget_elem = $("<div>").addClass("widget-content");
 
     widgets.get(widget_type)!.activate({
         $elem: $widget_elem,
@@ -114,15 +100,6 @@ export function activate(in_opts: WidgetOptions): void {
     }
 }
 
-export function set_widgets_for_list(): void {
-    for (const [idx, $widget_elem] of widget_contents) {
-        if (message_lists.current?.get(idx) !== undefined) {
-            const $row = message_lists.current.get_row(idx);
-            set_widget_in_message($row, $widget_elem);
-        }
-    }
-}
-
 export function handle_event(widget_event: Event & {message_id: number}): void {
     const $widget_elem = widget_contents.get(widget_event.message_id);
 

web/tests/widgetize.test.js

@@ -73,7 +73,7 @@ function test(label, f) {
     });
 }
 
-test("activate", ({override}) => {
+test("activate", () => {
     // Both widgetize.activate and widgetize.handle_event are tested
     // here to use the "caching" of widgets
     const $row = $.create("<stub message row>");
@@ -108,7 +108,6 @@ test("activate", ({override}) => {
     is_event_handled = false;
     assert.ok(!widgetize.widget_contents.has(opts.message.id));
 
-    $message_content.set_find_results(".widget-content", false);
     widgetize.activate(opts);
 
     assert.ok(is_widget_elem_inserted);
@@ -116,23 +115,11 @@ test("activate", ({override}) => {
     assert.ok(is_event_handled);
     assert.equal(widgetize.widget_contents.get(opts.message.id), $widget_elem);
 
-    is_widget_elem_inserted = false;
-    is_widget_activated = false;
-    is_event_handled = false;
-
-    $message_content.set_find_results(".widget-content", false);
-    widgetize.activate(opts);
-
-    assert.ok(is_widget_elem_inserted);
-    assert.ok(!is_widget_activated);
-    assert.ok(!is_event_handled);
-
     message_lists.current = undefined;
     is_widget_elem_inserted = false;
     is_widget_activated = false;
     is_event_handled = false;
 
-    $message_content.set_find_results(".widget-content", false);
     widgetize.activate(opts);
 
     assert.ok(!is_widget_elem_inserted);
@@ -179,16 +166,4 @@ test("activate", ({override}) => {
     post_activate_event.message_id = 1000;
     widgetize.handle_event(post_activate_event);
     assert.ok(!is_event_handled);
-
-    message_lists.current = {id: 2};
-    /* Test narrow change message update */
-    override(message_lists.current, "get", (idx) => {
-        assert.equal(idx, 2001);
-        return {};
-    });
-    override(message_lists.current, "get_row", (idx) => {
-        assert.equal(idx, 2001);
-        return $row;
-    });
-    widgetize.set_widgets_for_list();
 });