fix: editing text inside live preview leaks phoenix internal attributes

Author: devvaannsh

src/LiveDevelopment/BrowserScripts/LiveDevProtocolRemote.js

@@ -395,7 +395,7 @@
         var element = event.target;
         if (element && element.hasAttribute('data-brackets-id')) {
             // Check if it's a double-click for direct editing
-            if (event.detail === 2 && element.textContent && !['INPUT', 'TEXTAREA', 'SELECT'].includes(element.tagName)) {
+            if (event.detail === 2 && !['INPUT', 'TEXTAREA', 'SELECT'].includes(element.tagName)) {
                 // Double-click detected, enable direct editing
                 // Make the element editable
                 if (window._LD && window._LD.DOMEditHandler) {

src/LiveDevelopment/BrowserScripts/RemoteFunctions.js

@@ -1518,9 +1518,7 @@ function RemoteFunctions(config) {
         element.setAttribute("contenteditable", "true");
         element.focus();
 
-        // Save the original content for potential cancellation
         element._originalContent = element.innerHTML;
-        element._originalTextContent = element.textContent;
 
         // Add event listeners for editing
         function onBlur() {
@@ -1550,51 +1548,29 @@ function RemoteFunctions(config) {
         };
     }
 
-    // this function is to remove the properties from elements before getting the innerHTML
+    // this function is to remove the internal properties from elements before getting the innerHTML
     // then add all the properties back to the elements
+    // internal properties such as 'data-brackets-id', 'data-ld-highlight' etc
     function cleanupElementProperties(element) {
-        // a temporary container to hold a clean copy
-        const tempContainer = document.createElement('div');
-        tempContainer.innerHTML = element.innerHTML;
-
-        // get all elements in the temporary container
-        const allElements = tempContainer.querySelectorAll('*');
-
-        // Store original attributes for later restoration
-        const originalAttributes = new Map();
-        const elementsInOriginal = element.querySelectorAll('*');
-
-        // Save original attributes
-        elementsInOriginal.forEach((el, index) => {
-            const attrs = {};
-            for (let i = 0; i < el.attributes.length; i++) {
-                const attr = el.attributes[i];
-                attrs[attr.name] = attr.value;
-            }
-            originalAttributes.set(index, attrs);
-        });
+        const clone = element.cloneNode(true);
+        const allElements = [clone, ...clone.querySelectorAll('*')];
 
-        // Remove all attributes from elements in the temp container
         allElements.forEach(el => {
-            while (el.attributes.length > 0) {
-                el.removeAttribute(el.attributes[0].name);
+            // Remove Phoenix internal attributes
+            if (el.hasAttribute('data-brackets-id')) {
+                el.removeAttribute('data-brackets-id');
+            }
+            if (el.hasAttribute('data-ld-highlight')) {
+                el.removeAttribute('data-ld-highlight');
             }
-        });
-
-        // Get the clean HTML content
-        const cleanContent = tempContainer.innerHTML;
 
-        // Restore original attributes to the actual elements
-        elementsInOriginal.forEach((el, index) => {
-            if (originalAttributes.has(index)) {
-                const attrs = originalAttributes.get(index);
-                for (const [name, value] of Object.entries(attrs)) {
-                    el.setAttribute(name, value);
-                }
+            // Remove empty style attribute
+            if (el.hasAttribute('style') && el.getAttribute('style').trim() === '') {
+                el.removeAttribute('style');
             }
         });
 
-        return cleanContent;
+        return clone.innerHTML;
     }
 
     // Function to finish editing and apply changes
@@ -1622,8 +1598,6 @@ function RemoteFunctions(config) {
             window._Brackets_MessageBroker.send({
                 livePreviewEditEnabled: true,
                 element: element,
-                oldContent: element._originalContent,
-                oldTextContent: element._originalTextContent,
                 newContent: newContent,
                 tagId: Number(tagId),
                 livePreviewTextEdit: true
@@ -1632,7 +1606,6 @@ function RemoteFunctions(config) {
 
         // Clean up
         delete element._originalContent;
-        delete element._originalTextContent;
     }
 
     // init

src/LiveDevelopment/livePreviewEdit.js

@@ -2,6 +2,25 @@ define(function (require, exports, module) {
     const HTMLInstrumentation = require("LiveDevelopment/MultiBrowserImpl/language/HTMLInstrumentation");
     const LiveDevMultiBrowser = require("LiveDevelopment/LiveDevMultiBrowser");
 
+    /**
+     * this is a helper function to find the content boundaries in HTML
+     * @param {string} html - The HTML string to parse
+     * @return {Object} - Object with openTag and closeTag properties
+     */
+    function _findContentBoundaries(html) {
+        const openTagEnd = html.indexOf('>') + 1;
+        const closeTagStart = html.lastIndexOf('<');
+
+        if (openTagEnd > 0 && closeTagStart > openTagEnd) {
+            return {
+                openTag: html.substring(0, openTagEnd),
+                closeTag: html.substring(closeTagStart)
+            };
+        }
+
+        return null;
+    }
+
     /**
      * this function handles the text edit in the source code when user updates the text in the live preview
      * @param {Object} message - the message object
@@ -18,40 +37,32 @@ define(function (require, exports, module) {
     * join the text back and add the new content in between
     */
     function _editTextInSource(message) {
-        // this is to get the currently live document that is being served in the live preview
         const currLiveDoc = LiveDevMultiBrowser.getCurrentLiveDoc();
-        if(!currLiveDoc) {
+        if(!currLiveDoc || !currLiveDoc.editor || !message.tagId) {
             return;
         }
 
         const editor = currLiveDoc.editor;
-        if (!editor || !message.tagId) {
-            return;
-        }
-
-        // this will give us the start pos and end pos of the DOM element in the source code
-        // can be referenced using range.from and range.to
         const range = HTMLInstrumentation.getPositionFromTagId(editor, message.tagId);
         if (!range) {
             return;
         }
 
-        // this is the actual source code for the element that we need to duplicate
         const text = editor.getTextBetween(range.from, range.to);
-        // remove the <b>, <i> and <u> tags from the text,
-        // this is done because we split the text using the textContent and not the innerHTML
-        // and textContent doesn't have all these tags
-        const cleanedText = text.replace(/<\/?(b|i|u)>/gi, "");
-
-        // split the text as we want to remove the old content from the source code
-        // for ex: if we have <h1>hello</h1> then splitting from hello will give us [<h1>, </h1>]
-        const splittedText = cleanedText.split(message.oldTextContent);
-
-        // make sure that the split was successful
-        if (splittedText.length === 2) {
-            // so now we just merge the whole thing back replacing the old text content with the new one
+        let splittedText;
+
+        // we need to find the content boundaries to find exactly where the content starts and where it ends
+        const boundaries = _findContentBoundaries(text);
+        if (boundaries) {
+            splittedText = [boundaries.openTag, boundaries.closeTag];
+        }
+
+        // if the text split was done successfully, apply the edit
+        if (splittedText && splittedText.length === 2) {
             const finalText = splittedText[0] + message.newContent + splittedText[1];
             editor.replaceRange(finalText, range.from, range.to);
+        } else {
+            console.error("Live preview text edit operation failed.");
         }
     }