Always call `_super` when using Ember hooks

no issue - review use of Ember core hooks and add a call to `this._super` if missing - fix a few occurrences of using the wrong component lifecycle hooks that could result in multiple/duplicate event handlers being attached `_super` should always be called when overriding Ember's base hooks so that core functionality or app functionality added through extensions, mixins or addons is not lost. This is important as it guards against issues arising from later refactorings or core changes. As example of lost functionality, there were a number of routes that extended from `AuthenticatedRoute` but then overrode the `beforeModel` hook without calling `_super` which meant that the route was no longer treated as authenticated.
2015-11-15 11:06:49 +00:00 · 2015-11-15 11:06:49 +00:00 · 17b3ced8c9
parent 7ce3726589
commit 17b3ced8c9
42 changed files with 87 additions and 14 deletions
--- a/app/components/gh-cm-editor.js
+++ b/app/components/gh-cm-editor.js
@ -18,6 +18,8 @@ export default Component.extend({
    _editor: null, // reference to CodeMirror editor

    didInsertElement() {
+        this._super(...arguments);
+
        let options = this.getProperties('lineNumbers', 'indentUnit', 'mode', 'theme');
        let editor = new CodeMirror(this.get('element'), options);

@ -36,6 +38,8 @@ export default Component.extend({
    },

    willDestroyElement() {
+        this._super(...arguments);
+
        let editor = this._editor.getWrapperElement();
        editor.parentNode.removeChild(editor);
        this._editor = null;
--- a/app/components/gh-content-preview-content.js
+++ b/app/components/gh-content-preview-content.js
@ -9,6 +9,8 @@ export default Component.extend({
    content: null,

    didInsertElement() {
+        this._super(...arguments);
+
        let el = this.$();

        el.on('scroll', run.bind(el, setScrollClassName, {
@ -18,6 +20,8 @@ export default Component.extend({
    },

    didReceiveAttrs(options) {
+        this._super(...arguments);
+
        // adjust when didReceiveAttrs gets both newAttrs and oldAttrs
        if (options.newAttrs.content && this.get('content') !== options.newAttrs.content.value) {
            let el = this.$();
@ -29,6 +33,8 @@ export default Component.extend({
    },

    willDestroyElement() {
+        this._super(...arguments);
+
        let el = this.$();

        el.off('scroll');
--- a/app/components/gh-ed-preview.js
+++ b/app/components/gh-ed-preview.js
@ -9,12 +9,15 @@ export default Component.extend({
    _scrollWrapper: null,

    didInsertElement() {
+        this._super(...arguments);
        this._scrollWrapper = this.$().closest('.entry-preview-content');
        this.adjustScrollPosition(this.get('scrollPosition'));
        run.scheduleOnce('afterRender', this, this.dropzoneHandler);
    },

    didReceiveAttrs(attrs) {
+        this._super(...arguments);
+
        if (!attrs.oldAttrs) {
            return;
        }
--- a/app/components/gh-editor.js
+++ b/app/components/gh-editor.js
@ -44,6 +44,7 @@ export default Component.extend({
    },

    didInsertElement() {
+        this._super(...arguments);
        this.scheduleAfterRender();
    },

@ -61,6 +62,7 @@ export default Component.extend({
    },

    willDestroyElement() {
+        this._super(...arguments);
        // removes scroll handlers from the view
        this.get('$previewViewPort').off('scroll');
    },
--- a/app/components/gh-infinite-scroll-box.js
+++ b/app/components/gh-infinite-scroll-box.js
@ -5,7 +5,7 @@ import setScrollClassName from 'ghost/utils/set-scroll-classname';
 const {Component, run} = Ember;

 export default Component.extend(InfiniteScrollMixin, {
-    didRender() {
+    didInsertElement() {
        let el = this.$();

        this._super(...arguments);
--- a/app/components/gh-modal-dialog.js
+++ b/app/components/gh-modal-dialog.js
@ -34,6 +34,7 @@ export default Component.extend({
    }),

    didInsertElement() {
+        this._super(...arguments);
        this.$('.js-modal-container, .js-modal-background').addClass('fade-in open');
        this.$('.js-modal').addClass('open');
    },
--- a/app/components/gh-navigation.js
+++ b/app/components/gh-navigation.js
@ -31,6 +31,7 @@ export default Component.extend({
    },

    willDestroyElement() {
+        this._super(...arguments);
        this.$('.ui-sortable').sortable('destroy');
    }
 });
--- a/app/components/gh-navitem-url-input.js
+++ b/app/components/gh-navitem-url-input.js
@ -31,6 +31,8 @@ export default TextField.extend({
    }),

    didReceiveAttrs() {
+        this._super(...arguments);
+
        let baseUrl = this.get('baseUrl');
        let url = this.get('url');

--- a/app/components/gh-notification.js
+++ b/app/components/gh-notification.js
@ -30,6 +30,8 @@ export default Component.extend({
    }),

    didInsertElement() {
+        this._super(...arguments);
+
        this.$().on('animationend webkitAnimationEnd oanimationend MSAnimationEnd', (event) => {
            if (event.originalEvent.animationName === 'fade-out') {
                this.get('notifications').closeNotification(this.get('message'));
@ -38,6 +40,7 @@ export default Component.extend({
    },

    willDestroyElement() {
+        this._super(...arguments);
        this.$().off('animationend webkitAnimationEnd oanimationend MSAnimationEnd');
    },

--- a/app/components/gh-posts-list-item.js
+++ b/app/components/gh-posts-list-item.js
@ -42,10 +42,12 @@ export default Component.extend({
    },

    didInsertElement() {
+        this._super(...arguments);
        this.addObserver('active', this, this.scrollIntoView);
    },

    willDestroyElement() {
+        this._super(...arguments);
        this.removeObserver('active', this, this.scrollIntoView);
    },

--- a/app/components/gh-profile-image.js
+++ b/app/components/gh-profile-image.js
@ -48,6 +48,7 @@ export default Component.extend({
    },

    didReceiveAttrs(attrs) {
+        this._super(...arguments);
        let timeout = parseInt(attrs.newAttrs.throttle || this.get('debounce'));
        run.debounce(this, 'trySetValidEmail', timeout);
    },
@ -68,6 +69,8 @@ export default Component.extend({
        let size = this.get('size');
        let uploadElement = this.$('.js-file-input');

+        this._super(...arguments);
+
        // while theoretically the 'add' and 'processalways' functions could be
        // added as properties of the hash passed to fileupload(), for some reason
        // they needed to be placed in an on() call for the add method to work correctly
@ -85,6 +88,8 @@ export default Component.extend({
    },

    willDestroyElement() {
+        this._super(...arguments);
+
        if (this.$('.js-file-input').data()['blueimp-fileupload']) {
            this.$('.js-file-input').fileupload('destroy');
        }
--- a/app/components/gh-search-input.js
+++ b/app/components/gh-search-input.js
@ -117,6 +117,7 @@ export default Component.extend({
    },

    willDestroy() {
+        this._super(...arguments);
        this._resetKeymasterScope();
    },

--- a/app/components/gh-spin-button.js
+++ b/app/components/gh-spin-button.js
@ -53,6 +53,7 @@ export default Component.extend({
    }),

    willDestroy() {
+        this._super(...arguments);
        run.cancel(this.get('showSpinnerTimeout'));
    }
 });
--- a/app/components/gh-tab-pane.js
+++ b/app/components/gh-tab-pane.js
@ -21,11 +21,13 @@ export default Component.extend({
    active: alias('tab.active'),

    willRender() {
+        this._super(...arguments);
        // Register with the tabs manager
        this.get('tabsManager').registerTabPane(this);
    },

    willDestroyElement() {
+        this._super(...arguments);
        // Deregister with the tabs manager
        this.get('tabsManager').unregisterTabPane(this);
    }
--- a/app/components/gh-tab.js
+++ b/app/components/gh-tab.js
@ -22,11 +22,13 @@ export default Component.extend({
    },

    willRender() {
+        this._super(...arguments);
        // register the tabs with the tab manager
        this.get('tabsManager').registerTab(this);
    },

    willDestroyElement() {
+        this._super(...arguments);
        // unregister the tabs with the tab manager
        this.get('tabsManager').unregisterTab(this);
    }
--- a/app/components/gh-tag-settings-form.js
+++ b/app/components/gh-tag-settings-form.js
@ -77,6 +77,8 @@ export default Component.extend({
    }),

    didReceiveAttrs(attrs) {
+        this._super(...arguments);
+
        if (get(attrs, 'newAttrs.tag.value.id') !== get(attrs, 'oldAttrs.tag.value.id')) {
            this.reset();
        }
--- a/app/components/gh-uploader.js
+++ b/app/components/gh-uploader.js
@ -28,6 +28,8 @@ export default Component.extend({
        let oldValue = attrs.oldAttrs && get(attrs.oldAttrs, 'image.value');
        let newValue = attrs.newAttrs && get(attrs.newAttrs, 'image.value');

+        this._super(...arguments);
+
        // always reset when we receive a blank image
        // - handles navigating to populated image from blank image
        if (isEmpty(newValue) && !isEmpty(oldValue)) {
@ -45,10 +47,12 @@ export default Component.extend({
    },

    didInsertElement() {
+        this._super(...arguments);
        this.send('initUploader');
    },

    willDestroyElement() {
+        this._super(...arguments);
        this.removeListeners();
    },

--- a/app/controllers/feature.js
+++ b/app/controllers/feature.js
@ -26,6 +26,8 @@ export default Controller.extend(PromiseProxyMixin, {
    }),

    init() {
+        this._super(...arguments);
+
        let promise = this.store.query('setting', {type: 'blog,theme'}).then((settings) => {
            return settings.get('firstObject');
        });
--- a/app/mixins/editor-base-route.js
+++ b/app/mixins/editor-base-route.js
@ -125,7 +125,7 @@ export default Mixin.create(styleBody, ShortcutsRoute, {
        model.set('scratch', model.get('markdown'));
        model.set('titleScratch', model.get('title'));

-        this._super(controller, model);
+        this._super(...arguments);

        if (tags) {
            // used to check if anything has changed in the editor
--- a/app/mixins/infinite-scroll.js
+++ b/app/mixins/infinite-scroll.js
@ -24,6 +24,8 @@ export default Mixin.create({
    },

    didInsertElement() {
+        this._super(...arguments);
+
        let el = this.get('element');

        el.onscroll = run.bind(this, this.checkScroll);
@ -34,6 +36,8 @@ export default Mixin.create({
    },

    willDestroyElement() {
+        this._super(...arguments);
+
        // turn off the scroll handler
        this.get('element').onscroll = null;
    }
--- a/app/mixins/shortcuts-route.js
+++ b/app/mixins/shortcuts-route.js
@ -78,12 +78,12 @@ export default Mixin.create({
    },

    activate() {
-        this._super();
+        this._super(...arguments);
        this.registerShortcuts();
    },

    deactivate() {
-        this._super();
+        this._super(...arguments);
        this.removeShortcuts();
    }
 });
--- a/app/routes/editor/edit.js
+++ b/app/routes/editor/edit.js
@ -41,6 +41,8 @@ export default AuthenticatedRoute.extend(base, {
    },

    afterModel(post) {
+        this._super(...arguments);
+
        return this.get('session.user').then((user) => {
            if (user.get('isAuthor') && !post.isAuthoredByUser(user)) {
                return this.replaceRoute('posts.index');
--- a/app/routes/editor/index.js
+++ b/app/routes/editor/index.js
@ -4,6 +4,7 @@ const {Route} = Ember;

 export default Route.extend({
    beforeModel() {
+        this._super(...arguments);
        this.transitionTo('editor.new');
    }
 });
--- a/app/routes/editor/new.js
+++ b/app/routes/editor/new.js
@ -25,7 +25,7 @@ export default AuthenticatedRoute.extend(base, {
        });
    },

-    setupController(controller, model) {
+    setupController() {
        let psm = this.controllerFor('post-settings-menu');

        // make sure there are no titleObserver functions hanging around
@ -36,7 +36,7 @@ export default AuthenticatedRoute.extend(base, {
        psm.send('resetUploader');
        psm.send('resetPubDate');

-        this._super(controller, model);
+        this._super(...arguments);
    },

    actions: {
--- a/app/routes/mobile-index-route.js
+++ b/app/routes/mobile-index-route.js
@ -12,6 +12,7 @@ export default Route.extend({
    mediaQueries: inject.service(),

    activate() {
+        this._super(...arguments);
        this._callDesktopTransition = () => {
            if (!this.get('mediaQueries.isMobile')) {
                this.desktopTransition();
@ -21,6 +22,7 @@ export default Route.extend({
    },

    deactivate() {
+        this._super(...arguments);
        if (this._callDesktopTransition) {
            removeObserver(this, 'mediaQueries.isMobile', this._callDesktopTransition);
            this._callDesktopTransition = null;
--- a/app/routes/posts/index.js
+++ b/app/routes/posts/index.js
@ -13,6 +13,7 @@ export default MobileIndexRoute.extend(AuthenticatedRouteMixin, {

    // Transition to a specific post if we're not on mobile
    beforeModel() {
+        this._super(...arguments);
        if (!this.get('isMobile')) {
            return this.goToPost();
        }
@ -20,6 +21,7 @@ export default MobileIndexRoute.extend(AuthenticatedRouteMixin, {

    setupController(controller) {
        controller.set('noPosts', this.get('noPosts'));
+        this._super(...arguments);
    },

    goToPost() {
--- a/app/routes/reset.js
+++ b/app/routes/reset.js
@ -11,6 +11,7 @@ export default Route.extend(styleBody, {
    session: inject.service(),

    beforeModel() {
+        this._super(...arguments);
        if (this.get('session.isAuthenticated')) {
            this.get('notifications').showAlert('You can\'t reset your password while you\'re signed in.', {type: 'warn', delayed: true, key: 'password.reset.signed-in'});
            this.transitionTo(Configuration.routeAfterAuthentication);
@ -18,6 +19,7 @@ export default Route.extend(styleBody, {
    },

    setupController(controller, params) {
+        this._super(...arguments);
        controller.token = params.token;
    },

--- a/app/routes/settings/code-injection.js
+++ b/app/routes/settings/code-injection.js
@ -6,8 +6,8 @@ export default AuthenticatedRoute.extend(styleBody, CurrentUserSettings, {
    titleToken: 'Settings - Code Injection',
    classNames: ['settings-view-code'],

-    beforeModel(transition) {
-        this._super(transition);
+    beforeModel() {
+        this._super(...arguments);
        return this.get('session.user')
            .then(this.transitionAuthor())
            .then(this.transitionEditor());
--- a/app/routes/settings/general.js
+++ b/app/routes/settings/general.js
@ -7,8 +7,8 @@ export default AuthenticatedRoute.extend(styleBody, CurrentUserSettings, {

    classNames: ['settings-view-general'],

-    beforeModel(transition) {
-        this._super(transition);
+    beforeModel() {
+        this._super(...arguments);
        return this.get('session.user')
            .then(this.transitionAuthor())
            .then(this.transitionEditor());
--- a/app/routes/settings/labs.js
+++ b/app/routes/settings/labs.js
@ -7,8 +7,8 @@ export default AuthenticatedRoute.extend(styleBody, CurrentUserSettings, {

    classNames: ['settings'],

-    beforeModel(transition) {
-        this._super(transition);
+    beforeModel() {
+        this._super(...arguments);
        return this.get('session.user')
            .then(this.transitionAuthor())
            .then(this.transitionEditor());
--- a/app/routes/settings/navigation.js
+++ b/app/routes/settings/navigation.js
@ -7,8 +7,8 @@ export default AuthenticatedRoute.extend(styleBody, CurrentUserSettings, {

    classNames: ['settings-view-navigation'],

-    beforeModel(transition) {
-        this._super(transition);
+    beforeModel() {
+        this._super(...arguments);
        return this.get('session.user')
            .then(this.transitionAuthor());
    },
--- a/app/routes/settings/tags.js
+++ b/app/routes/settings/tags.js
@ -39,6 +39,7 @@ export default AuthenticatedRoute.extend(CurrentUserSettings, PaginationRoute, S
    },

    deactivate() {
+        this._super(...arguments);
        this.send('resetPagination');
    },

--- a/app/routes/settings/tags/index.js
+++ b/app/routes/settings/tags/index.js
@ -9,6 +9,9 @@ export default AuthenticatedRoute.extend({

    beforeModel() {
        let firstTag = this.modelFor('settings.tags').get('firstObject');
+
+        this._super(...arguments);
+
        if (firstTag && !this.get('mediaQueries.maxWidth600')) {
            this.transitionTo('settings.tags.tag', firstTag);
        }
--- a/app/routes/settings/tags/new.js
+++ b/app/routes/settings/tags/new.js
@ -14,6 +14,7 @@ export default AuthenticatedRoute.extend({

    // reset the model so that mobile screens react to an empty selectedTag
    deactivate() {
+        this._super(...arguments);
        this.set('controller.model', null);
    }

--- a/app/routes/settings/tags/tag.js
+++ b/app/routes/settings/tags/tag.js
@ -13,6 +13,7 @@ export default AuthenticatedRoute.extend({

    // reset the model so that mobile screens react to an empty selectedTag
    deactivate() {
+        this._super(...arguments);
        this.set('controller.model', null);
    }

--- a/app/routes/setup.js
+++ b/app/routes/setup.js
@ -16,6 +16,8 @@ export default Route.extend(styleBody, {
    // use the beforeModel hook to check to see whether or not setup has been
    // previously completed.  If it has, stop the transition into the setup page.
    beforeModel() {
+        this._super(...arguments);
+
        if (this.get('session.isAuthenticated')) {
            this.transitionTo(Configuration.routeIfAlreadyAuthenticated);
            return;
--- a/app/routes/setup/index.js
+++ b/app/routes/setup/index.js
@ -4,6 +4,7 @@ const {Route} = Ember;

 export default Route.extend({
    beforeModel() {
+        this._super(...arguments);
        this.transitionTo('setup.one');
    }
 });
--- a/app/routes/setup/one.js
+++ b/app/routes/setup/one.js
@ -9,6 +9,7 @@ let DownloadCountPoller = Ember.Object.extend({
    runId: null,

    init() {
+        this._super(...arguments);
        this.downloadCounter();
        this.poll();
    },
--- a/app/routes/setup/three.js
+++ b/app/routes/setup/three.js
@ -4,6 +4,7 @@ const {Route} = Ember;

 export default Route.extend({
    beforeModel() {
+        this._super(...arguments);
        if (!this.controllerFor('setup.two').get('blogCreated')) {
            this.transitionTo('setup.two');
        }
--- a/app/routes/signin.js
+++ b/app/routes/signin.js
@ -14,6 +14,8 @@ export default Route.extend(styleBody, {
    session: inject.service(),

    beforeModel() {
+        this._super(...arguments);
+
        if (this.get('session.isAuthenticated')) {
            this.transitionTo(Configuration.routeIfAlreadyAuthenticated);
        }
--- a/app/routes/signup.js
+++ b/app/routes/signup.js
@ -15,6 +15,8 @@ export default Route.extend(styleBody, {
    session: inject.service(),

    beforeModel() {
+        this._super(...arguments);
+
        if (this.get('session.isAuthenticated')) {
            this.get('notifications').showAlert('You need to sign out to register as a new user.', {type: 'warn', delayed: true, key: 'signup.create.already-authenticated'});
            this.transitionTo(Configuration.routeIfAlreadyAuthenticated);
--- a/app/routes/team/user.js
+++ b/app/routes/team/user.js
@ -17,6 +17,8 @@ export default AuthenticatedRoute.extend(styleBody, CurrentUserSettings, {
    },

    afterModel(user) {
+        this._super(...arguments);
+
        return this.get('session.user').then((currentUser) => {
            let isOwnProfile = user.get('id') === currentUser.get('id');
            let isAuthor = currentUser.get('isAuthor');