fix(icon): error when toggling icon with binding in IE11

* Fixes an error that was being logged by IE11 for icons that are toggled via `ngIf` and have a binding expression. * Some minor readability improvements in the icon component. Fixes angular#6093.
crisbeto · Aug 1, 2017 · 43482e4 · 43482e4
1 parent e79f7f9
commit 43482e4
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 14 deletions.
diff --git a/src/lib/icon/icon.spec.ts b/src/lib/icon/icon.spec.ts
@@ -47,6 +47,7 @@ describe('MdIcon', () => {
         IconWithCustomFontCss,
         IconFromSvgName,
         IconWithAriaHiddenFalse,
+        IconWithBindingAndNgIf,
       ],
       providers: [
         MockBackend,
@@ -313,6 +314,23 @@ describe('MdIcon', () => {
       verifyPathChildElement(svgElement, 'left');
       expect(svgElement.getAttribute('viewBox')).toBeFalsy();
     });
+
+    it('should not throw when toggling an icon that has a binding in IE11', () => {
+      mdIconRegistry.addSvgIcon('fluffy', trust('cat.svg'));
+
+      const fixture = TestBed.createComponent(IconWithBindingAndNgIf);
+
+      fixture.detectChanges();
+
+      expect(() => {
+        fixture.componentInstance.showIcon = false;
+        fixture.detectChanges();
+
+        fixture.componentInstance.showIcon = true;
+        fixture.detectChanges();
+      }).not.toThrow();
+    });
+
   });
 
   describe('custom fonts', () => {
@@ -405,3 +423,9 @@ class IconFromSvgName {
 
 @Component({template: '<md-icon aria-hidden="false">face</md-icon>'})
 class IconWithAriaHiddenFalse { }
+
+@Component({template: `<md-icon [svgIcon]="iconName" *ngIf="showIcon">{{iconName}}</md-icon>`})
+class IconWithBindingAndNgIf {
+  iconName = 'fluffy';
+  showIcon = true;
+}
diff --git a/src/lib/icon/icon.ts b/src/lib/icon/icon.ts
@@ -14,7 +14,7 @@ import {
   OnChanges,
   OnInit,
   Renderer2,
-  SimpleChange,
+  SimpleChanges,
   ViewEncapsulation,
   Attribute,
 } from '@angular/core';
@@ -134,17 +134,16 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo
     }
   }
 
-  ngOnChanges(changes: {[propertyName: string]: SimpleChange}) {
-    const changedInputs = Object.keys(changes);
+  ngOnChanges(changes: SimpleChanges) {
     // Only update the inline SVG icon if the inputs changed, to avoid unnecessary DOM operations.
-    if (changedInputs.indexOf('svgIcon') != -1 || changedInputs.indexOf('svgSrc') != -1) {
-      if (this.svgIcon) {
-        const [namespace, iconName] = this._splitIconName(this.svgIcon);
-        first.call(this._mdIconRegistry.getNamedSvgIcon(iconName, namespace)).subscribe(
-            svg => this._setSvgElement(svg),
-            (err: Error) => console.log(`Error retrieving icon: ${err.message}`));
-      }
+    if ((changes.svgIcon || changes.svgSrc) && this.svgIcon) {
+      const [namespace, iconName] = this._splitIconName(this.svgIcon);
+
+      first.call(this._mdIconRegistry.getNamedSvgIcon(iconName, namespace)).subscribe(
+          svg => this._setSvgElement(svg),
+          (err: Error) => console.log(`Error retrieving icon: ${err.message}`));
     }
+
     if (this._usingFontIcon()) {
       this._updateFontIconClasses();
     }
@@ -164,21 +163,26 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo
 
   private _setSvgElement(svg: SVGElement) {
     const layoutElement = this._elementRef.nativeElement;
-    // Remove existing child nodes and add the new SVG element.
-    // We would use renderer.detachView(Array.from(layoutElement.childNodes)) here,
-    // but it fails in IE11: https://github.com/angular/angular/issues/6327
-    layoutElement.innerHTML = '';
+
+    // Remove existing child nodes and add the new SVG element. Note that we can't
+    // use innerHTML, because IE will throw if the element has a data binding.
+    for (let child of layoutElement.childNodes) {
+      this._renderer.removeChild(layoutElement, child);
+    }
+
     this._renderer.appendChild(layoutElement, svg);
   }
 
   private _updateFontIconClasses() {
     if (!this._usingFontIcon()) {
       return;
     }
+
     const elem = this._elementRef.nativeElement;
     const fontSetClass = this.fontSet ?
         this._mdIconRegistry.classNameForFontAlias(this.fontSet) :
         this._mdIconRegistry.getDefaultFontSetClass();
+
     if (fontSetClass != this._previousFontSetClass) {
       if (this._previousFontSetClass) {
         this._renderer.removeClass(elem, this._previousFontSetClass);