In-progress updates for PR comments.

src/components/icon/icon-registry.ts

@@ -31,6 +31,9 @@ class SvgIconConfig {
   }
 }
 
+/** Returns the cache key to use for an icon namespace and name. */
+const iconKey = (namespace: string, name: string) => namespace + ':' + name;
+
 /**
  * Service to register and display icons used by the <md-icon> component.
  * - Registers icon URLs by namespace and name.
@@ -51,19 +54,13 @@ export class MdIconRegistry {
    */
   private _iconSetConfigs = new Map<string, SvgIconConfig[]>();
 
-  /**
-   * Cache for icons loaded by direct URLs.
-   */
+  /** Cache for icons loaded by direct URLs. */
   private _cachedIconsByUrl = new Map<string, SVGElement>();
 
-  /**
-   * In-progress icon fetches. Used to coalesce multiple requests to the same URL.
-   */
+  /** In-progress icon fetches. Used to coalesce multiple requests to the same URL. */
   private _inProgressUrlFetches = new Map<string, Observable<string>>();
 
-  /**
-   * Map from font identifiers to their CSS class names. Used for icon fonts.
-   */
+  /** Map from font identifiers to their CSS class names. Used for icon fonts. */
   private _fontCssClassesByAlias = new Map<string, string>();
 
   /**
@@ -75,32 +72,24 @@ export class MdIconRegistry {
 
   constructor(private _http: Http) {}
 
-  /**
-   * Registers an icon by URL in the default namespace.
-   */
+  /** Registers an icon by URL in the default namespace. */
   addSvgIcon(iconName: string, url: string): this {
     return this.addSvgIconInNamespace('', iconName, url);
   }
 
-  /**
-   * Registers an icon by URL in the specified namespace.
-   */
+  /** Registers an icon by URL in the specified namespace. */
   addSvgIconInNamespace(namespace: string, iconName: string, url: string): this {
-    const iconKey = namespace + ':' + iconName;
-    this._svgIconConfigs.set(iconKey, new SvgIconConfig(url));
+    const key = iconKey(namespace, iconName);
+    this._svgIconConfigs.set(key, new SvgIconConfig(url));
     return this;
   }
 
-  /**
-   * Registers an icon set by URL in the default namespace.
-   */
+  /** Registers an icon set by URL in the default namespace. */
   addSvgIconSet(url: string): this {
     return this.addSvgIconSetInNamespace('', url);
   }
 
-  /**
-   * Registers an icon set by URL in the specified namespace.
-   */
+  /** Registers an icon set by URL in the specified namespace. */
   addSvgIconSetInNamespace(namespace: string, url: string): this {
     const config = new SvgIconConfig(url);
     if (this._iconSetConfigs.has(namespace)) {
@@ -168,16 +157,16 @@ export class MdIconRegistry {
    */
   getNamedSvgIcon(name: string, namespace = ''): Observable<SVGElement> {
     // Return (copy of) cached icon if possible.
-    const iconKey = namespace + ':' + name;
-    if (this._svgIconConfigs.has(iconKey)) {
-      return this._getSvgFromConfig(this._svgIconConfigs.get(iconKey));
+    const key = iconKey(namespace, name);
+    if (this._svgIconConfigs.has(key)) {
+      return this._getSvgFromConfig(this._svgIconConfigs.get(key));
     }
     // See if we have any icon sets registered for the namespace.
     const iconSetConfigs = this._iconSetConfigs.get(namespace);
     if (iconSetConfigs) {
       return this._getSvgFromIconSetConfigs(name, this._iconSetConfigs.get(namespace));
     }
-    return Observable.throw(new MdIconNameNotFoundException(iconKey));
+    return Observable.throw(new MdIconNameNotFoundException(key));
   }
 
   /**
@@ -190,8 +179,8 @@ export class MdIconRegistry {
     } else {
       // Fetch the icon from the config's URL, cache it, and return a copy.
       return this._loadSvgIconFromConfig(config)
-          .do((svg: SVGElement) => config.svgElement = svg)
-          .map((svg: SVGElement) => svg.cloneNode(true));
+          .do(svg => config.svgElement = svg)
+          .map(svg => svg.cloneNode(true));
     }
   }
 
@@ -226,7 +215,7 @@ export class MdIconRegistry {
                   console.log(`Loading icon set URL: ${iconSetConfig.url} failed: ${err}`);
                   return Observable.of(null);
                 })
-                .do((svg: SVGElement) => {
+                .do(svg => {
                   // Cache SVG element.
                   if (svg) {
                     iconSetConfig.svgElement = svg;
@@ -278,6 +267,7 @@ export class MdIconRegistry {
    * from it.
    */
   private _loadSvgIconSetFromConfig(config: SvgIconConfig): Observable<SVGElement> {
+      // TODO: Document that icons should only be loaded from trusted sources.
     return this._fetchUrl(config.url)
         .map((svgText) => this._svgElementFromString(svgText));
   }

src/components/icon/icon.ts

@@ -1,7 +1,6 @@
 import {
   AfterContentChecked,
   ChangeDetectionStrategy,
-  ChangeDetectorRef,
   Component,
   ElementRef,
   Input,
@@ -12,10 +11,18 @@ import {
   ViewEncapsulation,
 } from 'angular2/core';
 import {NgClass} from 'angular2/common';
+import {BaseException} from 'angular2/src/facade/exceptions';
 
 import {MdIconRegistry} from './icon-registry';
 
 
+/** Exception thrown when an invalid icon name is passed to an md-icon component. */
+export class MdIconInvalidNameException extends BaseException {
+  constructor(iconName: string) {
+      super(`Invalid icon name: "${name}"`);
+  }
+}
+
 /**
  * Component to display an icon. It can be used in the following ways:
  * - Specify the svgSrc input to load an SVG icon from a URL. The SVG content is directly inlined
@@ -67,15 +74,14 @@ export class MdIcon implements OnChanges, OnInit, AfterContentChecked {
   @Input() fontIcon: string;
   @Input() alt: string;
 
-  @Input('aria-label') ariaLabelFromParent: string = '';
+  @Input('aria-label') hostAriaLabel: string = '';
 
   private _previousFontSetClass: string;
   private _previousFontIconClass: string;
 
   constructor(
       private _element: ElementRef,
       private _renderer: Renderer,
-      private _changeDetectorRef: ChangeDetectorRef,
       private _mdIconRegistry: MdIconRegistry) {
   }
 
@@ -85,20 +91,27 @@ export class MdIcon implements OnChanges, OnInit, AfterContentChecked {
    * The separator for the two fields is ':'. If there is no separator, an empty
    * string is returned for the icon set and the entire value is returned for
    * the icon name. If the argument is falsy, returns an array of two empty strings.
+   * Throws a MdIconInvalidNameException if the name contains two or more ':' separators.
    * Examples:
    *   'social:cake' -> ['social', 'cake']
    *   'penguin' -> ['', 'penguin']
    *   null -> ['', '']
+   *   'a:b:c' -> (throws MdIconInvalidNameException)
    */
   private _splitIconName(iconName: string): [string, string] {
     if (!iconName) {
       return ['', ''];
     }
-    const separatorIndex = this.svgIcon.indexOf(':');
-    if (separatorIndex == -1) {
-      return ['', iconName];
+    const parts = iconName.split(':');
+    switch (parts.length) {
+      case 1:
+        // Use default namespace.
+        return ['', parts[0]];
+      case 2:
+        return <[string, string]>parts;
+      default:
+        throw new MdIconInvalidNameException(iconName);
     }
-    return [iconName.substring(0, separatorIndex), iconName.substring(separatorIndex + 1)];
   }
 
   ngOnChanges(changes: { [propertyName: string]: SimpleChange }) {
@@ -140,7 +153,6 @@ export class MdIcon implements OnChanges, OnInit, AfterContentChecked {
       const ariaLabel = this._getAriaLabel();
       if (ariaLabel) {
         this._renderer.setElementAttribute(this._element.nativeElement, 'aria-label', ariaLabel);
-        this._changeDetectorRef.detectChanges();
       }
   }
 
@@ -149,7 +161,7 @@ export class MdIcon implements OnChanges, OnInit, AfterContentChecked {
     // reasonable value from the alt attribute, font icon name, SVG icon name, or (for ligatures)
     // the text content of the directive.
     const label =
-        this.ariaLabelFromParent ||
+        this.hostAriaLabel ||
         this.alt ||
         this.fontIcon ||
         this._splitIconName(this.svgIcon)[1];
@@ -163,7 +175,7 @@ export class MdIcon implements OnChanges, OnInit, AfterContentChecked {
         return text;
       }
     }
-    // Warn here?
+    // TODO: Warn here in dev mode.
     return null;
   }