Skip to content

fix(icon): make svg filters work in Safari/Firefox#11959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MikaAK wants to merge 3 commits into from
Closed

fix(icon): make svg filters work in Safari/Firefox #11959

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a389a0
fix(icon): make svg filters work in Safari/Firefox
MikaAK Jun 27, 2018
f39ead7
use location service instead of window.location
MikaAK Jun 27, 2018
a8e45ff
fix reference to location
MikaAK Jun 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions src/lib/icon/icon.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -18,6 +18,7 @@ import {
SimpleChanges,
ViewEncapsulation,
} from '@angular/core';
import {Location} from '@angular/common'
import {CanColor, mixinColor} from '@angular/material/core';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {MatIconRegistry} from './icon-registry';
Expand DownExpand Up@@ -112,6 +113,7 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Can

constructor(
elementRef: ElementRef,
private _location: Location,
private _iconRegistry: MatIconRegistry,
@Attribute('aria-hidden') ariaHidden: string) {
super(elementRef);
Expand DownExpand Up@@ -187,10 +189,12 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Can
// See: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/10898469/
// Do this before inserting the element into the DOM, in order to avoid a style recalculation.
const styleTags = svg.querySelectorAll('style') as NodeListOf<HTMLStyleElement>;

for (let i = 0; i < styleTags.length; i++) {
styleTags[i].textContent += ' ';
}

this._updateUrlPaths(svg.outerHTML);

this._elementRef.nativeElement.appendChild(svg);
}
Expand DownExpand Up@@ -236,7 +240,19 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Can
this._previousFontIconClass = this.fontIcon;
}
}


/**
* Updates the url paths with current paths append to icon which fixes
* SVG filters in Safari/Firefox
*/
private _updateUrlPaths(svg: SVGElement) {
const currentPath = this._location.prepareExternalUrl(this._location.path());

svg.outerHTML = svg.outerHTML.replace(/url\((.*)\)/, `url(${currentPath}$1)`);

@crisbetocrisbetoJun 30, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems very inefficient since it's asking the browser to parse the HTML once and then re-parse it once the URLs are updated. Since you're using a regex anyway, why not do it before the element is even created?

@MikaAKMikaAKJul 1, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regex does seem a bit brittle I agree. I'll admit I didn't read through the code at all, so I probably could of found a better spot. When I get a few minutes somehow I'll read the code and find a better spot for this!


return svg
}

/**
* Cleans up a value to be used as a fontIcon or fontSet.
* Since the value ends up being assigned as a CSS class, we
Expand Down