Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 3c7d36b

Browse files
committed
fix(ng-bind-html): deep watch the actual $sce value, not its toString.
Custom $sce implementations might not have a `toString()`, or it might be compiled away in non-debug mode. Deep watching the object is the correct fix with Angular's regular watch semantics. The performance of this should be equivalent - `toString()` on objects usually touches all fields, so a deep object watch will be in the same ballpark, except that it avoids the (potentially big) string allocation.
1 parent d71dc2f commit 3c7d36b

File tree

2 files changed

+33
-10
lines changed

2 files changed

+33
-10
lines changed

src/ng/directive/ngBind.js

+5-8
Original file line numberDiff line numberDiff line change
@@ -188,19 +188,16 @@ var ngBindHtmlDirective = ['$sce', '$parse', '$compile', function($sce, $parse,
188188
restrict: 'A',
189189
compile: function ngBindHtmlCompile(tElement, tAttrs) {
190190
var ngBindHtmlGetter = $parse(tAttrs.ngBindHtml);
191-
var ngBindHtmlWatch = $parse(tAttrs.ngBindHtml, function getStringValue(value) {
192-
return (value || '').toString();
193-
});
194191
$compile.$$addBindingClass(tElement);
195192

196193
return function ngBindHtmlLink(scope, element, attr) {
197194
$compile.$$addBindingInfo(element, attr.ngBindHtml);
198195

199-
scope.$watch(ngBindHtmlWatch, function ngBindHtmlWatchAction() {
200-
// we re-evaluate the expr because we want a TrustedValueHolderType
201-
// for $sce, not a string
202-
element.html($sce.getTrustedHtml(ngBindHtmlGetter(scope)) || '');
203-
});
196+
// Deep watch the value so that a getter that calls $sce.trustAsHtml and returns new objects
197+
// doesn't trigger infinite digest cycles.
198+
scope.$watch(ngBindHtmlGetter, function ngBindHtmlWatchAction(newVal) {
199+
element.html($sce.getTrustedHtml(newVal) || '');
200+
}, true);
204201
};
205202
}
206203
};

test/ng/directive/ngBindSpec.js

+28-2
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ describe('ngBind*', function() {
122122

123123
describe('ngBindHtml', function() {
124124

125-
it('should complain about accidental use of interpolation', inject(function($compile) {
125+
it('should complain about accidental use of interpolation', inject(function($rootScope, $compile) {
126126
expect(function() {
127127
$compile('<div ng-bind-html="{{myHtml}}"></div>');
128128
}).toThrowMinErr('$parse', 'syntax',
@@ -176,7 +176,7 @@ describe('ngBind*', function() {
176176
expect(angular.lowercase(element.html())).toEqual('<div onclick="">hello</div>');
177177
}));
178178

179-
it('should watch the string value to avoid infinite recursion', inject(function($rootScope, $compile, $sce) {
179+
it('should not cause infinite recursion for trustAsHtml object watches', inject(function($rootScope, $compile, $sce) {
180180
// Ref: https://github.com/angular/angular.js/issues/3932
181181
// If the binding is a function that creates a new value on every call via trustAs, we'll
182182
// trigger an infinite digest if we don't take care of it.
@@ -188,6 +188,32 @@ describe('ngBind*', function() {
188188
expect(angular.lowercase(element.html())).toEqual('<div onclick="">hello</div>');
189189
}));
190190

191+
it('should handle custom $sce objects', function() {
192+
function MySafeHtml(val) { this.val = val; }
193+
194+
module(function($provide) {
195+
$provide.decorator('$sce', function($delegate) {
196+
$delegate.trustAsHtml = function(html) { return new MySafeHtml(html); };
197+
$delegate.getTrustedHtml = function(mySafeHtml) { return mySafeHtml.val; };
198+
return $delegate;
199+
});
200+
});
201+
202+
inject(function($rootScope, $compile, $sce) {
203+
// Ref: https://github.com/angular/angular.js/issues/14526
204+
// Previous code used toString for change detection, which fails for custom objects
205+
// that don't override toString.
206+
element = $compile('<div ng-bind-html="getHtml()"></div>')($rootScope);
207+
var html = 'hello';
208+
$rootScope.getHtml = function() { return $sce.trustAsHtml(html); };
209+
$rootScope.$digest();
210+
expect(angular.lowercase(element.html())).toEqual('hello');
211+
html = 'goodbye';
212+
$rootScope.$digest();
213+
expect(angular.lowercase(element.html())).toEqual('goodbye');
214+
});
215+
});
216+
191217
describe('when $sanitize is available', function() {
192218
beforeEach(function() { module('ngSanitize'); });
193219

0 commit comments

Comments
 (0)