From 08d3db232cb758701c41277996f1b41773f98e0d Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Mon, 20 Jun 2022 09:08:31 +0200
Subject: [PATCH] fix(platform-server): invalid style attribute being generated
 for null 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
---
 packages/core/test/acceptance/styling_spec.ts | 54 +++++++------------
 .../platform-server/src/server_renderer.ts    | 16 ++++--
 2 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts
index 7fecb884fba..4a22d70fbe6 100644
--- a/packages/core/test/acceptance/styling_spec.ts
+++ b/packages/core/test/acceptance/styling_spec.ts
@@ -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', () => {
@@ -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
diff --git a/packages/platform-server/src/server_renderer.ts b/packages/platform-server/src/server_renderer.ts
index 7662ed145de..e3d112831f7 100644
--- a/packages/platform-server/src/server_renderer.ts
+++ b/packages/platform-server/src/server_renderer.ts
@@ -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);
   }
 
@@ -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');
+  }
 }