Skip to content
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.

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

Fix for #3123 (thanks to @anantanandgupta): use $templateRequest in templateFactory #3182

Merged
merged 5 commits into from
Dec 9, 2016

Conversation

rjamet
Copy link

@rjamet rjamet commented Dec 2, 2016

Based on PR #3155 from @anantanandgupta, fixing #3123 with more tests on top of it.

I've also updated the 1.3.x Angular used for tests to the latest one, 1.3.0 had a few $sce-related bugs. I got the Angular files from Google's CDN.

Finally, I'm not entirely sure this is the right way to send these commits, as I'm not too used to github: I can't propose commits to his PR, right? I'd be happy to send this another way if this is not the appropriate workflow :/

anantanandgupta and others added 5 commits November 16, 2016 05:31
- modified `$templateFactory` to use `$templateRequest` provider of
AngularJs in case it is avalable
- removed the test case which enforces the setting of request header
`Accept: text/html`
- AngularJS 1.0.8 don't have $injector.has
…on 1.3+

templateFactory uses templateRequest, which calls the $sce to run
the usual security checks on template URLs. This change verifies
that the policy is indeed enforced, and that the user can provide
$sce-trusted types through templateFactory (i.e., no assumptions
are made that URLs are plain strings).
@rjamet
Copy link
Author

rjamet commented Dec 5, 2016

Oops, missed the gruntfile. Fixed.

@anantanandgupta
Copy link

@rjamet Thanks for providing those tests. I guess that is no issue in that way of pull request as your commits are completely bases on my commits, if admins want to complete and pull my PR, they should be able to do it along with your commit.

@christopherthielen christopherthielen merged commit ad1f093 into angular-ui:legacy Dec 9, 2016
@christopherthielen
Copy link
Contributor

Looks good. Since this is a breaking change it will require a 0.4.0 release.
Before releasing that, I need a way to opt into the new $templateFactory behavior.
To do that, the service should be converted to a provider, which has $get() and has a useTemplateRequest() fn.

Then users who do not want to keep using $http can call:

.config($templateFactory => $templateFactory.useTemplateRequest(false));`

Last but not least, this should be merged into master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants