fix(dialog): leaking MdDialogContainer references

crisbeto · crisbeto · commit b0a9fde9d527 · 2017-02-24T23:25:47.000+01:00
Fixes an issue that caused the `MdDialogContainer` references to not be cleaned up, and as a result, any of the `MdDialogRef` and `MdDialogConfig` instances as well. The issue seems to come from the fact that a couple of blocks that look like: ``` this._ngZone.onMicrotaskEmpty.first().subscribe(() => { this._elementFocusedBeforeDialogWasOpened = document.activeElement; this._focusTrap.focusFirstTabbableElement(); }); ``` End up being transpiled to: ``` var _this = this; this._ngZone.onMicrotaskEmpty.first().subscribe(function () { _this._elementFocusedBeforeDialogWasOpened = document.activeElement; _this._focusTrap.focusFirstTabbableElement(); }); ``` This seems to cause the browser to retain the `_this` reference after the dialog has been destroyed. Fixes #2876.
diff --git a/src/lib/dialog/dialog-container.ts b/src/lib/dialog/dialog-container.ts
@@ -109,10 +109,8 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy {
     // If were to attempt to focus immediately, then the content of the dialog would not yet be
     // ready in instances where change detection has to run first. To deal with this, we simply
     // wait for the microtask queue to be empty.
-    this._ngZone.onMicrotaskEmpty.first().subscribe(() => {
-      this._elementFocusedBeforeDialogWasOpened = document.activeElement as HTMLElement;
-      this._focusTrap.focusFirstTabbableElement();
-    });
+    this._elementFocusedBeforeDialogWasOpened = document.activeElement as HTMLElement;
+    this._focusTrap.focusFirstTabbableElementWhenReady();
   }
 
   /**
@@ -137,16 +135,15 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy {
     // the dialog was opened. Wait for the DOM to finish settling before changing the focus so
     // that it doesn't end up back on the <body>. Also note that we need the extra check, because
     // IE can set the `activeElement` to null in some cases.
-    this._ngZone.onMicrotaskEmpty.first().subscribe(() => {
-      let toFocus = this._elementFocusedBeforeDialogWasOpened as HTMLElement;
+    let toFocus = this._elementFocusedBeforeDialogWasOpened as HTMLElement;
+    let animationStream = this._onAnimationStateChange;
 
-      // We need to check whether the focus method exists at all, because IE seems to throw an
-      // exception, even if the element is the document.body.
+    this._ngZone.onMicrotaskEmpty.first().subscribe(() => {
       if (toFocus && 'focus' in toFocus) {
         toFocus.focus();
       }
 
-      this._onAnimationStateChange.complete();
+      animationStream.complete();
     });
   }
 }
