FIX: prevents appEvents to leak (#7714)

app/assets/javascripts/discourse/components/d-modal.js.es6

@@ -35,42 +35,13 @@ export default Ember.Component.extend({
       }
     });
 
-    this.appEvents.on("modal:body-shown", data => {
-      if (this.isDestroying || this.isDestroyed) {
-        return;
-      }
-
-      if (data.fixed) {
-        this.$().removeClass("hidden");
-      }
-
-      if (data.title) {
-        this.set("title", I18n.t(data.title));
-      } else if (data.rawTitle) {
-        this.set("title", data.rawTitle);
-      }
-
-      if (data.subtitle) {
-        this.set("subtitle", I18n.t(data.subtitle));
-      } else if (data.rawSubtitle) {
-        this.set("subtitle", data.rawSubtitle);
-      } else {
-        // if no subtitle provided, makes sure the previous subtitle
-        // of another modal is not used
-        this.set("subtitle", null);
-      }
-
-      if ("dismissable" in data) {
-        this.set("dismissable", data.dismissable);
-      } else {
-        this.set("dismissable", true);
-      }
-    });
+    this.appEvents.on("modal:body-shown", this, "_modalBodyShown");
   },
 
   @on("willDestroyElement")
   cleanUp() {
     $("html").off("keydown.discourse-modal");
+    this.appEvents.off("modal:body-shown", this, "_modalBodyShown");
   },
 
   mouseDown(e) {
@@ -87,5 +58,37 @@ export default Ember.Component.extend({
       // the backdrop and makes it unclickable.
       $(".modal-header a.close").click();
     }
+  },
+
+  _modalBodyShown(data) {
+    if (this.isDestroying || this.isDestroyed) {
+      return;
+    }
+
+    if (data.fixed) {
+      this.$().removeClass("hidden");
+    }
+
+    if (data.title) {
+      this.set("title", I18n.t(data.title));
+    } else if (data.rawTitle) {
+      this.set("title", data.rawTitle);
+    }
+
+    if (data.subtitle) {
+      this.set("subtitle", I18n.t(data.subtitle));
+    } else if (data.rawSubtitle) {
+      this.set("subtitle", data.rawSubtitle);
+    } else {
+      // if no subtitle provided, makes sure the previous subtitle
+      // of another modal is not used
+      this.set("subtitle", null);
+    }
+
+    if ("dismissable" in data) {
+      this.set("dismissable", data.dismissable);
+    } else {
+      this.set("dismissable", true);
+    }
   }
 });

app/assets/javascripts/discourse/components/mount-widget.js.es6

@@ -61,7 +61,7 @@ export default Ember.Component.extend({
   willDestroyElement() {
     this._dispatched.forEach(evt => {
       const [eventName, caller] = evt;
-      this.appEvents.off(eventName, caller);
+      this.appEvents.off(eventName, this, caller);
     });
     Ember.run.cancel(this._timeout);
   },
@@ -84,7 +84,7 @@ export default Ember.Component.extend({
     const caller = refreshArg =>
       this.eventDispatched(eventName, key, refreshArg);
     this._dispatched.push([eventName, caller]);
-    this.appEvents.on(eventName, caller);
+    this.appEvents.on(eventName, this, caller);
   },
 
   queueRerender(callback) {

app/assets/javascripts/discourse/controllers/topic.js.es6

@@ -108,22 +108,32 @@ export default Ember.Controller.extend(bufferedProperty("model"), {
 
   init() {
     this._super(...arguments);
-    this.appEvents.on("post:show-revision", (postNumber, revision) => {
-      const post = this.model.get("postStream").postForPostNumber(postNumber);
-      if (!post) {
-        return;
-      }
 
-      Ember.run.scheduleOnce("afterRender", () => {
-        this.send("showHistory", post, revision);
-      });
-    });
+    this.appEvents.on("post:show-revision", this, "_showRevision");
+
     this.setProperties({
       selectedPostIds: [],
       quoteState: new QuoteState()
     });
   },
 
+  willDestroy() {
+    this._super(...arguments);
+
+    this.appEvents.off("post:show-revision", this, "_showRevision");
+  },
+
+  _showRevision(postNumber, revision) {
+    const post = this.model.get("postStream").postForPostNumber(postNumber);
+    if (!post) {
+      return;
+    }
+
+    Ember.run.scheduleOnce("afterRender", () => {
+      this.send("showHistory", post, revision);
+    });
+  },
+
   showCategoryChooser: Ember.computed.not("model.isPrivateMessage"),
 
   gotoInbox(name) {

app/assets/javascripts/discourse/initializers/avatar-select.js.es6

@@ -5,27 +5,32 @@ export default {
   name: "avatar-select",
 
   initialize(container) {
-    const siteSettings = container.lookup("site-settings:main");
-    const appEvents = container.lookup("app-events:main");
+    this.selectAvatarsEnabled = container.lookup(
+      "site-settings:main"
+    ).select_avatars_enabled;
 
-    appEvents.on("show-avatar-select", user => {
-      const avatarTemplate = user.get("avatar_template");
-      let selected = "uploaded";
+    container
+      .lookup("app-events:main")
+      .on("show-avatar-select", this, "_showAvatarSelect");
+  },
 
-      if (avatarTemplate === user.get("system_avatar_template")) {
-        selected = "system";
-      } else if (avatarTemplate === user.get("gravatar_avatar_template")) {
-        selected = "gravatar";
-      }
+  _showAvatarSelect(user) {
+    const avatarTemplate = user.avatar_template;
+    let selected = "uploaded";
 
-      const modal = showModal("avatar-selector");
-      modal.setProperties({ user, selected });
+    if (avatarTemplate === user.system_avatar_template) {
+      selected = "system";
+    } else if (avatarTemplate === user.gravatar_avatar_template) {
+      selected = "gravatar";
+    }
 
-      if (siteSettings.selectable_avatars_enabled) {
-        ajax("/site/selectable-avatars.json").then(avatars =>
-          modal.set("selectableAvatars", avatars)
-        );
-      }
-    });
+    const modal = showModal("avatar-selector");
+    modal.setProperties({ user, selected });
+
+    if (this.selectAvatarsEnabled) {
+      ajax("/site/selectable-avatars.json").then(avatars =>
+        modal.set("selectableAvatars", avatars)
+      );
+    }
   }
 };

app/assets/javascripts/discourse/initializers/badging.js.es6

@@ -4,16 +4,20 @@ export default {
   after: "message-bus",
 
   initialize(container) {
-    const appEvents = container.lookup("app-events:main");
-    const user = container.lookup("current-user:main");
-
-    if (!user) return; // must be logged in
     if (!window.ExperimentalBadge) return; // must have the Badging API
 
-    appEvents.on("notifications:changed", () => {
-      let notifications =
-        user.get("unread_notifications") + user.get("unread_private_messages");
-      window.ExperimentalBadge.set(notifications);
-    });
+    const user = container.lookup("current-user:main");
+    if (!user) return; // must be logged in
+
+    this.notifications =
+      user.unread_notifications + user.unread_private_messages;
+
+    container
+      .lookup("app-events:main")
+      .on("notifications:changed", this, "_updateBadge");
+  },
+
+  _updateBadge() {
+    window.ExperimentalBadge.set(this.notifications);
   }
 };

app/assets/javascripts/discourse/initializers/title-notifications.js.es6

@@ -3,16 +3,18 @@ export default {
   after: "message-bus",
 
   initialize(container) {
-    const appEvents = container.lookup("app-events:main");
     const user = container.lookup("current-user:main");
-
     if (!user) return; // must be logged in
 
-    appEvents.on("notifications:changed", () => {
-      let notifications =
-        user.get("unread_notifications") + user.get("unread_private_messages");
+    this.notifications =
+      user.unread_notifications + user.unread_private_messages;
 
-      Discourse.updateNotificationCount(notifications);
-    });
+    container
+      .lookup("app-events:main")
+      .on("notifications:changed", this, "_updateTitle");
+  },
+
+  _updateTitle() {
+    Discourse.updateNotificationCount(this.notifications);
   }
 };

app/assets/javascripts/discourse/mixins/card-contents-base.js.es6

@@ -127,10 +127,16 @@ export default Ember.Mixin.create({
 
     this.appEvents.on(previewClickEvent, this, "_previewClick");
 
-    this.appEvents.on(`topic-header:trigger-${id}`, (username, $target) => {
-      this.setProperties({ isFixed: true, isDocked: true });
-      return this._show(username, $target);
-    });
+    this.appEvents.on(
+      `topic-header:trigger-${id}`,
+      this,
+      "_topicHeaderTrigger"
+    );
+  },
+
+  _topicHeaderTrigger(username, $target) {
+    this.setProperties({ isFixed: true, isDocked: true });
+    return this._show(username, $target);
   },
 
   _bindMobileScroll() {
@@ -281,7 +287,14 @@ export default Ember.Mixin.create({
     $("#main")
       .off(clickDataExpand)
       .off(clickMention);
+
     this.appEvents.off(previewClickEvent, this, "_previewClick");
+
+    this.appEvents.off(
+      `topic-header:trigger-${this.elementId}`,
+      this,
+      "_topicHeaderTrigger"
+    );
   },
 
   keyUp(e) {

test/javascripts/test_helper.js

@@ -154,6 +154,18 @@ QUnit.testDone(function() {
   flushMap();
 
   server.shutdown();
+
+  // ensures any event not removed is not leaking between tests
+  // most like in intialisers, other places (controller, component...)
+  // should be fixed in code
+  var appEvents = window.Discourse.__container__.lookup("app-events:main");
+  var events = appEvents.__proto__._events;
+  Object.keys(events).forEach(function(eventKey) {
+    var event = events[eventKey];
+    event.forEach(function(listener) {
+      appEvents.off(eventKey, listener.target, listener.fn);
+    });
+  });
 });
 
 // Load ES6 tests