diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts index ec90b57830..6ebfca3c3c 100644 --- a/packages/core/src/render3/instructions/styling.ts +++ b/packages/core/src/render3/instructions/styling.ts @@ -13,7 +13,7 @@ import {RElement} from '../interfaces/renderer'; import {StylingMapArray, StylingMapArrayIndex, TStylingConfig, TStylingContext} from '../interfaces/styling'; import {isDirectiveHost} from '../interfaces/type_checks'; import {BINDING_INDEX, LView, RENDERER} from '../interfaces/view'; -import {getActiveDirectiveId, getCurrentStyleSanitizer, getLView, getSelectedIndex, setCurrentStyleSanitizer, setElementExitFn} from '../state'; +import {getActiveDirectiveId, getCurrentStyleSanitizer, getLView, getSelectedIndex, resetCurrentStyleSanitizer, setCurrentStyleSanitizer, setElementExitFn} from '../state'; import {applyStylingMapDirectly, applyStylingValueDirectly, flushStyling, setClass, setStyle, updateClassViaContext, updateStyleViaContext} from '../styling/bindings'; import {activateStylingMapFeature} from '../styling/map_based_bindings'; import {attachStylingDebugObject} from '../styling/styling_debug'; @@ -174,10 +174,18 @@ function stylingProp( // Direct Apply Case: bypass context resolution and apply the // style/class value directly to the element if (allowDirectStyling(context, hostBindingsMode)) { + const sanitizerToUse = isClassBased ? null : sanitizer; const renderer = getRenderer(tNode, lView); updated = applyStylingValueDirectly( renderer, context, native, lView, bindingIndex, prop, value, isClassBased, - isClassBased ? setClass : setStyle, sanitizer); + isClassBased ? setClass : setStyle, sanitizerToUse); + + if (sanitizerToUse) { + // it's important we remove the current style sanitizer once the + // element exits, otherwise it will be used by the next styling + // instructions for the next element. + setElementExitFn(resetCurrentStyleSanitizer); + } } else { // Context Resolution (or first update) Case: save the value // and defer to the context to flush and apply the style/class binding @@ -337,10 +345,17 @@ function _stylingMap( // Direct Apply Case: bypass context resolution and apply the // style/class map values directly to the element if (allowDirectStyling(context, hostBindingsMode)) { + const sanitizerToUse = isClassBased ? null : sanitizer; const renderer = getRenderer(tNode, lView); updated = applyStylingMapDirectly( renderer, context, native, lView, bindingIndex, stylingMapArr as StylingMapArray, - isClassBased, isClassBased ? setClass : setStyle, sanitizer, valueHasChanged); + isClassBased, isClassBased ? setClass : setStyle, sanitizerToUse, valueHasChanged); + if (sanitizerToUse) { + // it's important we remove the current style sanitizer once the + // element exits, otherwise it will be used by the next styling + // instructions for the next element. + setElementExitFn(resetCurrentStyleSanitizer); + } } else { updated = valueHasChanged; activateStylingMapFeature(); @@ -442,7 +457,7 @@ function stylingApply(): void { const classesContext = isStylingContext(tNode.classes) ? tNode.classes as TStylingContext : null; const stylesContext = isStylingContext(tNode.styles) ? tNode.styles as TStylingContext : null; flushStyling(renderer, lView, classesContext, stylesContext, native, directiveIndex, sanitizer); - setCurrentStyleSanitizer(null); + resetCurrentStyleSanitizer(); } function getRenderer(tNode: TNode, lView: LView) { diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index ce7d5d2a67..0c3897ee00 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -500,6 +500,10 @@ export function setCurrentStyleSanitizer(sanitizer: StyleSanitizeFn | null) { _currentSanitizer = sanitizer; } +export function resetCurrentStyleSanitizer() { + setCurrentStyleSanitizer(null); +} + export function getCurrentStyleSanitizer() { return _currentSanitizer; } diff --git a/packages/core/src/render3/styling/bindings.ts b/packages/core/src/render3/styling/bindings.ts index f1ef045ddd..b4145ec23a 100644 --- a/packages/core/src/render3/styling/bindings.ts +++ b/packages/core/src/render3/styling/bindings.ts @@ -756,7 +756,7 @@ function applyStylingValue( let valueToApply: string|null = unwrapSafeValue(value); if (isStylingValueDefined(valueToApply)) { valueToApply = - sanitizer ? sanitizer(prop, value, StyleSanitizeMode.SanitizeOnly) : valueToApply; + sanitizer ? sanitizer(prop, value, StyleSanitizeMode.ValidateAndSanitize) : valueToApply; applyFn(renderer, element, prop, valueToApply, bindingIndex); return true; } @@ -771,8 +771,9 @@ function findAndApplyMapValue( const p = getMapProp(map, i); if (p === prop) { let valueToApply = getMapValue(map, i); - valueToApply = - sanitizer ? sanitizer(prop, valueToApply, StyleSanitizeMode.SanitizeOnly) : valueToApply; + valueToApply = sanitizer ? + sanitizer(prop, valueToApply, StyleSanitizeMode.ValidateAndSanitize) : + valueToApply; applyFn(renderer, element, prop, valueToApply, bindingIndex); return true; } diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index 072c44a7d7..1a6c4a2d65 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -2117,6 +2117,62 @@ describe('styling', () => { expect(div.nativeElement.style['width']).toEqual('200px'); } }); + + it('should not set classes when falsy value is passed while a sanitizer is present', () => { + @Component({ + // Note that we use `background` here because it needs to be sanitized. + template: ` + +
+ `, + }) + + class AppComponent { + isDisabled = false; + background = 'orange'; + } + + TestBed.configureTestingModule({declarations: [AppComponent]}); + const fixture = TestBed.createComponent(AppComponent); + fixture.detectChanges(); + + const span = fixture.nativeElement.querySelector('span'); + expect(span.classList).not.toContain('disabled'); + + // The issue we're testing for happens after the second change detection. + fixture.detectChanges(); + expect(span.classList).not.toContain('disabled'); + }); + + it('should not set classes when falsy value is passed while a sanitizer from host bindings is present', + () => { + @Directive({selector: '[blockStyles]'}) + class StylesDirective { + @HostBinding('style.border') + border = '1px solid red'; + + @HostBinding('style.background') + background = 'white'; + } + + @Component({ + template: `
`, + }) + class AppComponent { + isDisabled = false; + } + + TestBed.configureTestingModule({declarations: [AppComponent, StylesDirective]}); + const fixture = TestBed.createComponent(AppComponent); + fixture.detectChanges(); + + const div = fixture.nativeElement.querySelector('div'); + expect(div.classList.contains('disabled')).toBe(false); + + // The issue we're testing for happens after the second change detection. + fixture.detectChanges(); + expect(div.classList.contains('disabled')).toBe(false); + }); }); function assertStyleCounters(countForSet: number, countForRemove: number) { diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 80cce44a4a..443c9b5b7d 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -1154,6 +1154,9 @@ { "name": "resetComponentState" }, + { + "name": "resetCurrentStyleSanitizer" + }, { "name": "resetPreOrderHookFlags" },