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
wants to merge 3 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ca25a0
feat(tooltip): use expression to fix usage with $sce
chrisirhc Apr 19, 2015
8c37865
feat(popover): use expression to fix usage with $sce
chrisirhc Apr 19, 2015
9164908
fix(tooltip): tooltip-html should not open if empty
chrisirhc Apr 19, 2015
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
2 changes: 1 addition & 1 deletion src/popover/docs/demo.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ <h4>Dynamic</h4>
</div>
<button popover="{{dynamicPopover.content}}" popover-title="{{dynamicPopover.title}}" class="btn btn-default">Dynamic Popover</button>

<button popover-template="{{dynamicPopover.templateUrl}}" popover-title="{{dynamicPopover.title}}" class="btn btn-default">Popover With Template</button>
<button popover-template="dynamicPopover.templateUrl" popover-title="{{dynamicPopover.title}}" class="btn btn-default">Popover With Template</button>

<script type="text/ng-template" id="myPopoverTemplate.html">
<div>{{dynamicPopover.content}}</div>
Expand Down
6 changes: 4 additions & 2 deletions src/popover/popover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@ angular.module( 'ui.bootstrap.popover', [ 'ui.bootstrap.tooltip' ] )
return {
restrict: 'EA',
replace: true,
scope: { title: '@', content: '@', placement: '@', animation: '&', isOpen: '&',
scope: { title: '@', contentExp: '&', placement: '@', animation: '&', isOpen: '&',
originScope: '&' },
templateUrl: 'template/popover/popover-template.html'
};
})

.directive( 'popoverTemplate', [ '$tooltip', function ( $tooltip ) {
return $tooltip( 'popoverTemplate', 'popover', 'click' );
return $tooltip( 'popoverTemplate', 'popover', 'click', {
useContentExp: true
} );
}])

.directive( 'popoverPopup', function () {
Expand Down
16 changes: 15 additions & 1 deletion src/popover/test/popover-template.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('popover template', function() {

beforeEach(inject(function($rootScope, $compile) {
elmBody = angular.element(
'<div><span popover-template="{{ templateUrl }}">Selector Text</span></div>'
'<div><span popover-template="templateUrl">Selector Text</span></div>'
);

scope = $rootScope;
Expand Down Expand Up @@ -62,5 +62,19 @@ describe('popover template', function() {

expect( elmBody.children().eq(1).text().trim() ).toBe( 'new text' );
}));

it('should hide popover when template becomes empty', inject(function ($timeout) {
elm.trigger( 'click' );
expect( tooltipScope.isOpen ).toBe( true );

scope.templateUrl = '';
scope.$digest();

expect( tooltipScope.isOpen ).toBe( false );

$timeout.flush();
expect( elmBody.children().length ).toBe( 1 );
}));

});

2 changes: 1 addition & 1 deletion src/tooltip/docs/demo.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).

nunc sed velit dignissim sodales ut eu sem integer vitae. Turpis egestas
</p>

Expand Down
16 changes: 15 additions & 1 deletion src/tooltip/test/tooltip-template.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('tooltip template', function() {

beforeEach(inject(function($rootScope, $compile) {
elmBody = angular.element(
'<div><span tooltip-template="{{ templateUrl }}">Selector Text</span></div>'
'<div><span tooltip-template="templateUrl">Selector Text</span></div>'
);

scope = $rootScope;
Expand Down Expand Up @@ -61,5 +61,19 @@ describe('tooltip template', function() {

expect( elmBody.children().eq(1).text().trim() ).toBe( 'new text' );
}));

it('should hide tooltip when template becomes empty', inject(function ($timeout) {
elm.trigger( 'mouseenter' );
expect( tooltipScope.isOpen ).toBe( true );

scope.templateUrl = '';
scope.$digest();

expect( tooltipScope.isOpen ).toBe( false );

$timeout.flush();
expect( elmBody.children().length ).toBe( 1 );
}));

});

7 changes: 7 additions & 0 deletions src/tooltip/test/tooltip.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,13 @@ describe( 'tooltipHtml', function() {
expect( elmBody.find('.tooltip-inner').html() ).toBe( scope.html );
}));

it( 'should not open if html is empty', function () {
scope.safeHtml = null;
scope.$digest();
elm.trigger( 'mouseenter' );
expect( tooltipScope.isOpen ).toBe( false );
});

it( 'should show on mouseenter and hide on mouseleave', inject( function ($sce) {
expect( tooltipScope.isOpen ).toBe( false );

Expand Down
46 changes: 31 additions & 15 deletions src/tooltip/tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
var defaultOptions = {
placement: 'top',
animation: true,
popupDelay: 0
popupDelay: 0,
useContentExp: false
};

// Default hide triggers for each show trigger
Expand Down Expand Up @@ -65,8 +66,8 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
* TODO support multiple triggers
*/
this.$get = [ '$window', '$compile', '$timeout', '$document', '$position', '$interpolate', function ( $window, $compile, $timeout, $document, $position, $interpolate ) {
return function $tooltip ( type, prefix, defaultTriggerShow ) {
var options = angular.extend( {}, defaultOptions, globalOptions );
return function $tooltip ( type, prefix, defaultTriggerShow, options ) {
options = angular.extend( {}, defaultOptions, globalOptions, options );

/**
* Returns an object of show and hide triggers.
Expand Down Expand Up @@ -98,8 +99,9 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
var template =
'<div '+ directiveName +'-popup '+
'title="'+startSym+'title'+endSym+'" '+
'content="'+startSym+'content'+endSym+'" '+
'content-exp="contentExp()" '+
(options.useContentExp ?
'content-exp="contentExp()" ' :
'content="'+startSym+'content'+endSym+'" ') +
'placement="'+startSym+'placement'+endSym+'" '+
'popup-class="'+startSym+'popupClass'+endSym+'" '+
'animation="animation" '+
Expand Down Expand Up @@ -188,7 +190,7 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
}

// Don't show empty tooltips.
if ( ! ttScope.content ) {
if ( !(options.useContentExp ? ttScope.contentExp() : ttScope.content) ) {
return angular.noop;
}

Expand Down Expand Up @@ -247,6 +249,14 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
tooltipLinkedScope.$watch(function () {
$timeout(positionTooltip, 0, false);
});

if (options.useContentExp) {
tooltipLinkedScope.$watch('contentExp()', function (val) {
if (!val && ttScope.isOpen ) {
hide();
}
});
}
}

function removeTooltip() {
Expand Down Expand Up @@ -274,13 +284,15 @@ angular.module( 'ui.bootstrap.tooltip', [ 'ui.bootstrap.position', 'ui.bootstrap
/**
* Observe the relevant attributes.
*/
attrs.$observe( type, function ( val ) {
ttScope.content = val;
if (!options.useContentExp) {
attrs.$observe( type, function ( val ) {
ttScope.content = val;

if (!val && ttScope.isOpen ) {
hide();
}
});
if (!val && ttScope.isOpen ) {
hide();
}
});
}

attrs.$observe( 'disabled', function ( val ) {
if (val && ttScope.isOpen ) {
Expand Down Expand Up @@ -466,14 +478,16 @@ function ($animate , $sce , $compile , $templateRequest) {
return {
restrict: 'EA',
replace: true,
scope: { title: '@', content: '@', placement: '@', animation: '&', isOpen: '&',
scope: { title: '@', contentExp: '&', placement: '@', animation: '&', isOpen: '&',
originScope: '&' },
templateUrl: 'template/tooltip/tooltip-template-popup.html'
};
})

.directive( 'tooltipTemplate', [ '$tooltip', function ( $tooltip ) {
return $tooltip( 'tooltipTemplate', 'tooltip', 'mouseenter' );
return $tooltip('tooltipTemplate', 'tooltip', 'mouseenter', {
useContentExp: true
});
}])

.directive( 'tooltipHtmlPopup', function () {
Expand All @@ -486,7 +500,9 @@ function ($animate , $sce , $compile , $templateRequest) {
})

.directive( 'tooltipHtml', [ '$tooltip', function ( $tooltip ) {
return $tooltip( 'tooltipHtml', 'tooltip', 'mouseenter' );
return $tooltip('tooltipHtml', 'tooltip', 'mouseenter', {
useContentExp: true
});
}])

/*
Expand Down
2 changes: 1 addition & 1 deletion template/popover/popover-template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<div class="popover-inner">
<h3 class="popover-title" ng-bind="title" ng-if="title"></h3>
<div class="popover-content"
tooltip-template-transclude="content"
tooltip-template-transclude="contentExp()"
tooltip-template-transclude-scope="originScope()"></div>
</div>
</div>
2 changes: 1 addition & 1 deletion template/tooltip/tooltip-template-popup.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
ng-class="{ in: isOpen() }">
<div class="tooltip-arrow"></div>
<div class="tooltip-inner"
tooltip-template-transclude="content"
tooltip-template-transclude="contentExp()"
tooltip-template-transclude-scope="originScope()"></div>
</div>