fix(icon): use SafeResourceUrl in getSvgIconFromUrl #7535
Conversation
googlebot
added
the
cla: yes
| @@ -201,7 +201,7 @@ export class MatIconRegistry { | |||
| return observableOf(cloneSvg(cachedIcon)); | |||
| } | |||
|
|
|||
| return RxChain.from(this._loadSvgIconFromConfig(new SvgIconConfig(url))) | |||
| return RxChain.from(this._loadSvgIconFromConfig(new SvgIconConfig(safeUrl))) | |||
There was a problem hiding this comment.
While we're at it, it might be a good idea to rename the url variable above to something along the lines of sanitizedUrl to avoid confusion in the future.
There was a problem hiding this comment.
Really the type should enforce passing the right thing around; I want to check with Martin about adding a brand to safe types (since they're empty they match everything)
crisbeto
added
pr: lgtm
action: merge
|
Looks like we still need to update a test here: https://travis-ci.org/angular/material2/jobs/283345030 |
|
Weird, failing only in IE11 |
src/lib/icon/icon.spec.ts
Outdated
|
|
||
| // Assert that a registered icon can be looked-up by url. | ||
| iconRegistry.getSvgIconFromUrl(trust('cat.svg')).subscribe(element => { | ||
| expect(element.innerHTML).toContain('meow'); |
There was a problem hiding this comment.
SVGs don't have innerHTML on IE. This should be verifyPathChildElement(element, 'meow');.
jelbourn
force-pushed
the
icon-safe-url
branch
from
f15eb45
to
5aadee7
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.