fix(dialog): leaking MdDialogContainer references

crisbeto · crisbeto · commit edc342d232b7 · 2017-02-05T20:20:28.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
@@ -5,7 +5,6 @@ import {
   ViewEncapsulation,
   NgZone,
   OnDestroy,
-  Renderer,
 } from '@angular/core';
 import {BasePortalHost, ComponentPortal, PortalHostDirective, TemplatePortal} from '../core';
 import {MdDialogConfig} from './dialog-config';
@@ -47,7 +46,7 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy {
   /** Reference to the open dialog. */
   dialogRef: MdDialogRef<any>;
 
-  constructor(private _ngZone: NgZone, private _renderer: Renderer) {
+  constructor(private _ngZone: NgZone) {
     super();
   }
 
@@ -65,10 +64,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;
-      this._focusTrap.focusFirstTabbableElement();
-    });
+    this._elementFocusedBeforeDialogWasOpened = document.activeElement;
+    this._focusTrap.focusFirstTabbableElementWhenReady();
 
     return attachResult;
   }
@@ -93,10 +90,10 @@ 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.
-    if (this._elementFocusedBeforeDialogWasOpened) {
-      this._ngZone.onMicrotaskEmpty.first().subscribe(() => {
-        this._renderer.invokeElementMethod(this._elementFocusedBeforeDialogWasOpened, 'focus');
-      });
+    let toFocus = this._elementFocusedBeforeDialogWasOpened as HTMLElement;
+
+    if (toFocus && 'focus' in toFocus) {
+      this._ngZone.onMicrotaskEmpty.first().subscribe(() => toFocus.focus());
     }
   }
 }
