Skip to content

Commit

Permalink
fix(platform-server): invalid style attribute being generated for nul…
Browse files Browse the repository at this point in the history
…l values (#46433)

Fixes that the server renderer was producing an invalid `style` attribute when a null value is passed in. Also aligns the behavior with the DOM renderer by removing the attribute when it's empty.

Fixes #46385.

PR Close #46433
  • Loading branch information
crisbeto authored and AndrewKushnir committed Jun 21, 2022
1 parent 5d3b97e commit 6ad7b40
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 38 deletions.
54 changes: 20 additions & 34 deletions packages/core/test/acceptance/styling_spec.ts
Expand Up @@ -16,40 +16,6 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
import {expectPerfCounters} from '@angular/private/testing';

describe('styling', () => {
/**
* This helper function tests to see if the current browser supports non standard way of writing
* into styles.
*
* This is not the correct way to write to style and is not supported in IE11.
* ```
* div.style = 'color: white';
* ```
*
* This is the correct way to write to styles:
* ```
* div.style.cssText = 'color: white';
* ```
*
* Even though writing to `div.style` is not officially supported, it works in all
* browsers except IE11.
*
* This function detects this condition and allows us to skip affected tests.
*/
let _supportsWritingStringsToStyleProperty: boolean|null = null;
function supportsWritingStringsToStyleProperty() {
if (_supportsWritingStringsToStyleProperty === null) {
const div = document.createElement('div');
const CSS = 'color: white;';
try {
(div as any).style = CSS;
} catch (e) {
_supportsWritingStringsToStyleProperty = false;
}
_supportsWritingStringsToStyleProperty = (div.style.cssText === CSS);
}
return _supportsWritingStringsToStyleProperty;
}

beforeEach(ngDevModeResetPerfCounters);

describe('apply in prioritization order', () => {
Expand Down Expand Up @@ -1406,6 +1372,26 @@ describe('styling', () => {
expect(element.classList.contains('dir-two')).toBeTruthy();
});

it('should not write empty style values to the DOM', () => {
@Component({
template: `
<div
[style.color]="null"
[style.--bg-color]="undefined"
[style.margin]="''"
[style.font-size]="' '"></div>
`
})
class Cmp {
}

TestBed.configureTestingModule({declarations: [Cmp]});
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();

expect(fixture.nativeElement.innerHTML).toBe('<div></div>');
});

describe('NgClass', () => {
// We had a bug where NgClass would not allocate sufficient slots for host bindings,
// so it would overwrite information about other directives nearby. This test checks
Expand Down
16 changes: 12 additions & 4 deletions packages/platform-server/src/server_renderer.ts
Expand Up @@ -155,11 +155,12 @@ class DefaultServerRenderer2 implements Renderer2 {

setStyle(el: any, style: string, value: any, flags: RendererStyleFlags2): void {
style = style.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
value = value == null ? '' : `${value}`.trim();
const styleMap = _readStyleAttribute(el);
if (flags & RendererStyleFlags2.Important) {
value += ' !important';
}
styleMap[style] = value == null ? '' : value;
styleMap[style] = value;
_writeStyleAttribute(el, styleMap);
}

Expand Down Expand Up @@ -297,12 +298,19 @@ function _readStyleAttribute(element: any): {[name: string]: string} {
}

function _writeStyleAttribute(element: any, styleMap: {[name: string]: string}) {
// We have to construct the `style` attribute ourselves, instead of going through
// `element.style.setProperty` like the other renderers, because `setProperty` won't
// write newer CSS properties that Domino doesn't know about like `clip-path`.
let styleAttrValue = '';
for (const key in styleMap) {
const newValue = styleMap[key];
if (newValue != null) {
styleAttrValue += key + ':' + styleMap[key] + ';';
if (newValue != null && newValue !== '') {
styleAttrValue += key + ':' + newValue + ';';
}
}
element.setAttribute('style', styleAttrValue);
if (styleAttrValue) {
element.setAttribute('style', styleAttrValue);
} else {
element.removeAttribute('style');
}
}

0 comments on commit 6ad7b40

Please sign in to comment.