From 2d91ef420363cfda22075ea14f27864e49dfd785 Mon Sep 17 00:00:00 2001
From: Oleg Isonen <oleg008@gmail.com>
Date: Mon, 29 Apr 2024 13:51:47 +0200
Subject: [PATCH] chore: don't fix boolean attributes to empty string values in
 html embed (#3251)

## Description

When validating html embed, dom innerHTML always turns boolean
attributes into empty string, e.g. <div hidden></div> will always become
<div hidden=""></div> and this is not necessary, we can ignore those
mismatches.


## Steps for reproduction

1. click button
2. expect xyz

## Code Review

- [ ] hi @kof, I need you to do
  - conceptual review (architecture, feature-correctness)
  - detailed review (read every line)
  - test it on preview

## Before requesting a review

- [ ] made a self-review
- [ ] added inline comments where things may be not obvious (the "why",
not "what")

## Before merging

- [ ] tested locally and on preview environment (preview dev login:
5de6)
- [ ] updated [test
cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md)
document
- [ ] added tests
- [ ] if any new env variables are added, added them to `.env.example`
and the `builder/env-check.js` if mandatory
---
 .../features/settings-panel/controls/code.tsx | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/apps/builder/app/builder/features/settings-panel/controls/code.tsx b/apps/builder/app/builder/features/settings-panel/controls/code.tsx
index 6609acf382..a2556d8e3d 100644
--- a/apps/builder/app/builder/features/settings-panel/controls/code.tsx
+++ b/apps/builder/app/builder/features/settings-panel/controls/code.tsx
@@ -63,15 +63,28 @@ const ErrorInfo = ({
 
 type Error = { message: string; value: string; expected: string };
 
+// The problem is to identify broken HTML and because browser is flexible and always tries to fix it we never
+// know if something is actually broken.
+// 1. Parse the HTML using DOM
+// 2. Get HTML via innerHTML
+// 3. Compare the original HTML with innerHTML
+// 4. We try to minimize the amount of false positives by removing
+//    - different amount of whitespace
+//    - unifying `boolean=""` is the same as `boolean`
 const validateHtml = (value: string): Error | undefined => {
-  // This is basically what browser does when innerHTML is set
-  // but isolated within temporary element
-  // so the result is correct markup
   const div = document.createElement("div");
   div.innerHTML = value;
   const expected = div.innerHTML;
-  // We don't need to show error for unnecessary whitespace.
-  if (value.replace(/\s/g, "") !== expected.replace(/\s/g, "")) {
+  const clean = (value: string) => {
+    return (
+      value
+        // Compare without whitespace to avoid false positives
+        .replace(/\s/g, "")
+        // normalize boolean attributes by turning `boolean=""` into `boolean`
+        .replace('=""', "")
+    );
+  };
+  if (clean(value) !== clean(expected)) {
     return { message: "Invalid HTML detected", value, expected };
   }
 };