Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

feat(tooltip/popover): fix usage with $sce #3563

Closed

Conversation

chrisirhc
Copy link
Contributor

Closes #3558

@chrisirhc chrisirhc added this to the 0.13.0 (AngularJS 1.3) milestone Apr 19, 2015
@chrisirhc chrisirhc changed the title feat(tooltip/popover): fix usage with $sce WIP feat(tooltip/popover): fix usage with $sce Apr 19, 2015
@karianna
Copy link
Contributor

@chrisirhc Tests fail...

Allows for trusted resource URLs through Strict Contextual Escaping ($sce).
If the an interpolated expression is used instead, then the benefits of SCE is
lost.

Fixes angular-ui#3558
Allows for trusted resource URLs through Strict Contextual Escaping ($sce).
If the an interpolated expression is used instead, then the benefits of SCE is
lost.

Fixes angular-ui#3558
@chrisirhc chrisirhc force-pushed the feature/fix-popover-template branch from 445ceb0 to 8c37865 Compare April 20, 2015 13:36
@chrisirhc
Copy link
Contributor Author

Thanks for checking @karianna , I marked it as "WIP" because of the failing tests. Just updated it.

@chrisirhc chrisirhc changed the title WIP feat(tooltip/popover): fix usage with $sce feat(tooltip/popover): fix usage with $sce Apr 20, 2015
@@ -20,7 +20,7 @@
<a href="#" tooltip-animation="false" tooltip="I don't fade. :-(">fading</a>
at elementum eu, facilisis sed odio morbi quis commodo odio. In cursus
<a href="#" tooltip-popup-delay='1000' tooltip='appears with delay'>delayed</a> turpis massa tincidunt dui ut.
<a href="#" tooltip-template="myTooltipTemplate.html">Custom template</a>
<a href="#" tooltip-template="'myTooltipTemplate.html'">Custom template</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain this change? This seems a little awkward for use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because if a user needs to use a trusted resource url, the user can now do the following.

In controller:

$scope.templateUrl = $sce.trustAsResourceUrl('http://someurl.com/template.htm');

In template tooltip-template="templateUrl".

Before, the if the user did tooltip-template="{{templateUrl}}, the URL gets converted into a plain string after interpolation and hence, loses its trusted context. Then there's no way for the user to specify a trusted URL. $sce trust isn't needed for URLs on the current security origin but will fail when the user specifies URLs from another security origin (different domain/protocol/port).

@wesleycho
Copy link
Contributor

This PR LGTM, just had a question about a change.

@chrisirhc
Copy link
Contributor Author

Both those Plunkers aren't using the popover directive. Did you press "Save" to make sure that the Plunker updated?

@chrisirhc
Copy link
Contributor Author

Please open another issue if this isn't related to this particular PR. We can talk about it there.
Let's keep discussion on PRs strictly about the changes in the PR itself.

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

Successfully merging this pull request may close these issues.

Change popover/tooltip-template to evaluate expression
4 participants