diff --git a/src/lib/icon/fake-svgs.ts b/src/lib/icon/fake-svgs.ts index 6d38254123ca..1148f2387584 100644 --- a/src/lib/icon/fake-svgs.ts +++ b/src/lib/icon/fake-svgs.ts @@ -8,39 +8,40 @@ /** * Fake URLs and associated SVG documents used by tests. + * The ID attribute is used to load the icons, the name attribute is only used for testing. * @docs-private */ export const FAKE_SVGS = { - cat: '', - dog: '', + cat: '', + dog: '', farmSet1: ` - - + + `, farmSet2: ` - - + + `, farmSet3: ` - - + + `, arrows: ` - - + + ` }; diff --git a/src/lib/icon/icon-registry.ts b/src/lib/icon/icon-registry.ts index 6d054db8a7f7..828ab35e18d3 100644 --- a/src/lib/icon/icon-registry.ts +++ b/src/lib/icon/icon-registry.ts @@ -365,23 +365,28 @@ export class MatIconRegistry { * returns it. Returns null if no matching element is found. */ private _extractSvgIconFromSet(iconSet: SVGElement, iconName: string): SVGElement | null { - const iconNode = iconSet.querySelector('#' + iconName); + const iconSource = iconSet.querySelector('#' + iconName); - if (!iconNode) { + if (!iconSource) { return null; } + // Clone the element and remove the ID to prevent multiple elements from being added + // to the page with the same ID. + const iconElement = iconSource.cloneNode(true) as Element; + iconElement.id = ''; + // If the icon node is itself an node, clone and return it directly. If not, set it as // the content of a new node. - if (iconNode.tagName.toLowerCase() === 'svg') { - return this._setSvgAttributes(iconNode.cloneNode(true) as SVGElement); + if (iconElement.nodeName.toLowerCase() === 'svg') { + return this._setSvgAttributes(iconElement as SVGElement); } // If the node is a , it won't be rendered so we have to convert it into . Note // that the same could be achieved by referring to it via , however the // tag is problematic on Firefox, because it needs to include the current page path. - if (iconNode.nodeName.toLowerCase() === 'symbol') { - return this._setSvgAttributes(this._toSvgElement(iconNode)); + if (iconElement.nodeName.toLowerCase() === 'symbol') { + return this._setSvgAttributes(this._toSvgElement(iconElement)); } // createElement('SVG') doesn't work as expected; the DOM ends up with @@ -391,7 +396,7 @@ export class MatIconRegistry { // http://stackoverflow.com/questions/23003278/svg-innerhtml-in-firefox-can-not-display const svg = this._svgElementFromString(''); // Clone the node so we don't remove it from the parent icon set element. - svg.appendChild(iconNode.cloneNode(true)); + svg.appendChild(iconElement); return this._setSvgAttributes(svg); } @@ -400,8 +405,6 @@ export class MatIconRegistry { * Creates a DOM element from the given SVG string. */ private _svgElementFromString(str: string): SVGElement { - // TODO: Is there a better way than innerHTML? Renderer doesn't appear to have a method for - // creating an element from an HTML string. const div = document.createElement('DIV'); div.innerHTML = str; const svg = div.querySelector('svg') as SVGElement; diff --git a/src/lib/icon/icon.spec.ts b/src/lib/icon/icon.spec.ts index 4b34fd0c0ebb..a3fa25d6770b 100644 --- a/src/lib/icon/icon.spec.ts +++ b/src/lib/icon/icon.spec.ts @@ -17,6 +17,7 @@ function sortedClassNames(element: Element): string[] { * Verifies that an element contains a single child element, and returns that child. */ function verifyAndGetSingleSvgChild(element: SVGElement): SVGElement { + expect(element.id).toBeFalsy(); expect(element.childNodes.length).toBe(1); const svgChild = element.childNodes[0] as SVGElement; expect(svgChild.tagName.toLowerCase()).toBe('svg'); @@ -31,7 +32,9 @@ function verifyPathChildElement(element: Element, attributeValue: string): void expect(element.childNodes.length).toBe(1); const pathElement = element.childNodes[0] as SVGPathElement; expect(pathElement.tagName.toLowerCase()).toBe('path'); - expect(pathElement.getAttribute('id')).toBe(attributeValue); + + // The testing data SVGs have the name attribute set for verification. + expect(pathElement.getAttribute('name')).toBe(attributeValue); } @@ -190,7 +193,7 @@ describe('MatIcon', () => { svgChild = svgElement.childNodes[0]; // The first child should be the element. expect(svgChild.tagName.toLowerCase()).toBe('g'); - expect(svgChild.getAttribute('id')).toBe('pig'); + expect(svgChild.getAttribute('name')).toBe('pig'); verifyPathChildElement(svgChild, 'oink'); // Change the icon, and the SVG element should be replaced. @@ -200,7 +203,7 @@ describe('MatIcon', () => { svgChild = svgElement.childNodes[0]; // The first child should be the element. expect(svgChild.tagName.toLowerCase()).toBe('g'); - expect(svgChild.getAttribute('id')).toBe('cow'); + expect(svgChild.getAttribute('name')).toBe('cow'); verifyPathChildElement(svgChild, 'moo'); }); @@ -224,7 +227,8 @@ describe('MatIcon', () => { svgChild = svgElement.childNodes[0]; // The child should be the element. expect(svgChild.tagName.toLowerCase()).toBe('g'); - expect(svgChild.getAttribute('id')).toBe('pig'); + expect(svgChild.getAttribute('name')).toBe('pig'); + expect(svgChild.getAttribute('id')).toBe(''); expect(svgChild.childNodes.length).toBe(1); verifyPathChildElement(svgChild, 'oink'); @@ -237,7 +241,7 @@ describe('MatIcon', () => { svgChild = svgElement.childNodes[0]; // The first child should be the element. expect(svgChild.tagName.toLowerCase()).toBe('g'); - expect(svgChild.getAttribute('id')).toBe('cow'); + expect(svgChild.getAttribute('name')).toBe('cow'); expect(svgChild.childNodes.length).toBe(1); verifyPathChildElement(svgChild, 'moo moo'); }); @@ -259,7 +263,7 @@ describe('MatIcon', () => { expect(svgElement.querySelector('symbol')).toBeFalsy(); expect(svgElement.childNodes.length).toBe(1); expect(firstChild.nodeName.toLowerCase()).toBe('path'); - expect((firstChild as HTMLElement).getAttribute('id')).toBe('quack'); + expect((firstChild as HTMLElement).getAttribute('name')).toBe('quack'); }); it('should not wrap elements in icon sets in another svg tag', () => { diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index 45cb03cf0c59..a58f331646b7 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -1870,7 +1870,6 @@ describe('MatSelect', () => { // There appears to be a small rounding error on IE, so we verify that the value is close, // not exact. - let platform = new Platform(); if (platform.TRIDENT) { let difference = Math.abs(optionTop + (menuItemHeight - triggerHeight) / 2 - triggerTop);