Skip to content

Commit e98c07e

Browse files
committed
fix: browser removing incorrect css syntax from code which is unintended behaviour
1 parent 92ed8b3 commit e98c07e

File tree

2 files changed

+138
-164
lines changed

2 files changed

+138
-164
lines changed

src/LiveDevelopment/BrowserScripts/RemoteFunctions.js

Lines changed: 7 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,57 +1905,6 @@ function RemoteFunctions(config) {
19051905
_editHandler.apply(edits);
19061906
}
19071907

1908-
/**
1909-
*
1910-
* @param {Element} elem
1911-
*/
1912-
function _domElementToJSON(elem) {
1913-
var json = { tag: elem.tagName.toLowerCase(), attributes: {}, children: [] },
1914-
i,
1915-
len,
1916-
node,
1917-
value;
1918-
1919-
len = elem.attributes.length;
1920-
for (i = 0; i < len; i++) {
1921-
node = elem.attributes.item(i);
1922-
1923-
// skip internal attributes that shouldn't be serialized
1924-
if (node.name === "draggable" && node.value === "true") {
1925-
continue;
1926-
}
1927-
value = (node.name === "data-brackets-id") ? parseInt(node.value, 10) : node.value;
1928-
// Clean internal style properties
1929-
if (node.name === "style") {
1930-
const cleanedStyle = _cleanInternalStyles(value);
1931-
1932-
if (cleanedStyle.trim() === '') {
1933-
continue; // Skip empty style attribute
1934-
}
1935-
value = cleanedStyle;
1936-
}
1937-
json.attributes[node.name] = value;
1938-
}
1939-
1940-
len = elem.childNodes.length;
1941-
for (i = 0; i < len; i++) {
1942-
node = elem.childNodes.item(i);
1943-
1944-
// ignores comment nodes and visuals generated by live preview
1945-
if (node.nodeType === Node.ELEMENT_NODE && node.className !== HIGHLIGHT_CLASSNAME) {
1946-
json.children.push(_domElementToJSON(node));
1947-
} else if (node.nodeType === Node.TEXT_NODE) {
1948-
json.children.push({ content: node.nodeValue });
1949-
}
1950-
}
1951-
1952-
return json;
1953-
}
1954-
1955-
function getSimpleDOM() {
1956-
return JSON.stringify(_domElementToJSON(window.document.documentElement));
1957-
}
1958-
19591908
function updateConfig(newConfig) {
19601909
var oldConfig = config;
19611910
config = JSON.parse(newConfig);
@@ -2056,9 +2005,6 @@ function RemoteFunctions(config) {
20562005

20572006
dismissMoreOptionsBox();
20582007

2059-
element._originalContent = cleanupElementProperties(element);
2060-
2061-
// Add event listeners for editing
20622008
function onBlur() {
20632009
finishEditing(element);
20642010
}
@@ -2067,7 +2013,7 @@ function RemoteFunctions(config) {
20672013
if (event.key === "Escape") {
20682014
// Cancel editing
20692015
event.preventDefault();
2070-
finishEditing(element);
2016+
finishEditing(element, false); // false means that the edit operation was cancelled
20712017
} else if (event.key === "Enter" && !event.shiftKey) {
20722018
// Finish editing on Enter (unless Shift is held)
20732019
event.preventDefault();
@@ -2085,69 +2031,9 @@ function RemoteFunctions(config) {
20852031
};
20862032
}
20872033

2088-
// Helper function to clean internal style properties
2089-
function _cleanInternalStyles(styleValue) {
2090-
if (typeof styleValue !== "string") {
2091-
return styleValue;
2092-
}
2093-
2094-
let cleanedStyle = styleValue;
2095-
2096-
// remove internal background color
2097-
cleanedStyle = cleanedStyle.replace(/background-color:\s*rgba\(0,\s*162,\s*255,\s*0\.2\)\s*;?\s*/gi, "");
2098-
2099-
// remove internal outline
2100-
cleanedStyle = cleanedStyle.replace(/outline:\s*rgb\(66,\s*133,\s*244\)\s+solid\s+1px\s*;?\s*/gi, "");
2101-
cleanedStyle = cleanedStyle.replace(/outline:\s*1px\s+solid\s+#4285F4\s*;?\s*/gi, "");
2102-
2103-
// clean up any extra spaces or semicolons
2104-
cleanedStyle = cleanedStyle
2105-
.replace(/;\s*;/g, ";")
2106-
.replace(/^\s*;\s*/, "")
2107-
.replace(/\s*;\s*$/, "");
2108-
2109-
return cleanedStyle;
2110-
}
2111-
2112-
// this function is to remove the internal properties from elements before getting the innerHTML
2113-
// then add all the properties back to the elements
2114-
// internal properties such as 'data-brackets-id', 'data-ld-highlight' etc
2115-
function cleanupElementProperties(element) {
2116-
const clone = element.cloneNode(true);
2117-
const allElements = [clone, ...clone.querySelectorAll('*')];
2118-
2119-
allElements.forEach(el => {
2120-
// Remove Phoenix internal attributes
2121-
if (el.hasAttribute('data-brackets-id')) {
2122-
el.removeAttribute('data-brackets-id');
2123-
}
2124-
if (el.hasAttribute('data-ld-highlight')) {
2125-
el.removeAttribute('data-ld-highlight');
2126-
}
2127-
2128-
// remove the draggable attribute added internally
2129-
if (el.hasAttribute('draggable')) {
2130-
el.removeAttribute('draggable');
2131-
}
2132-
2133-
// remove internal style properties
2134-
if (el.hasAttribute('style')) {
2135-
const style = el.getAttribute('style');
2136-
const cleanedStyle = _cleanInternalStyles(style);
2137-
2138-
if (cleanedStyle.trim() === '') {
2139-
el.removeAttribute('style');
2140-
} else {
2141-
el.setAttribute('style', cleanedStyle);
2142-
}
2143-
}
2144-
});
2145-
2146-
return clone.innerHTML;
2147-
}
2148-
21492034
// Function to finish editing and apply changes
2150-
function finishEditing(element) {
2035+
// isEditSuccessful: this is a boolean value, defaults to true. false only when the edit operation is cancelled
2036+
function finishEditing(element, isEditSuccessful = true) {
21512037
if (!element || !element.hasAttribute("contenteditable")) {
21522038
return;
21532039
}
@@ -2163,23 +2049,17 @@ function RemoteFunctions(config) {
21632049
delete element._editListeners;
21642050
}
21652051

2166-
// Get the new content after cleaning up unwanted properties
2167-
const newContent = cleanupElementProperties(element);
2168-
2169-
// If content has changed, send the edit to the editor
2170-
if (newContent !== element._originalContent && element.hasAttribute("data-brackets-id")) {
2052+
if (element.hasAttribute("data-brackets-id")) {
21712053
const tagId = element.getAttribute("data-brackets-id");
21722054
window._Brackets_MessageBroker.send({
21732055
livePreviewEditEnabled: true,
2056+
livePreviewTextEdit: true,
21742057
element: element,
2175-
newContent: newContent,
2058+
newContent: element.outerHTML,
21762059
tagId: Number(tagId),
2177-
livePreviewTextEdit: true
2060+
isEditSuccessful: isEditSuccessful
21782061
});
21792062
}
2180-
2181-
// Clean up
2182-
delete element._originalContent;
21832063
}
21842064

21852065
// init
@@ -2209,7 +2089,6 @@ function RemoteFunctions(config) {
22092089
"redrawHighlights" : redrawHighlights,
22102090
"redrawEverything" : redrawEverything,
22112091
"applyDOMEdits" : applyDOMEdits,
2212-
"getSimpleDOM" : getSimpleDOM,
22132092
"updateConfig" : updateConfig,
22142093
"startEditing" : startEditing,
22152094
"finishEditing" : finishEditing,

src/LiveDevelopment/LivePreviewEdit.js

Lines changed: 131 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,46 +4,139 @@ define(function (require, exports, module) {
44
const CodeMirror = require("thirdparty/CodeMirror/lib/codemirror");
55

66
/**
7-
* this is a helper function to find the content boundaries in HTML
8-
* @param {string} html - The HTML string to parse
9-
* @return {Object} - Object with openTag and closeTag properties
7+
* This function is to sync text content changes between the original source code
8+
* and the live preview DOM after a text edit operation
9+
*
10+
* @param {String} oldContent - the original source code from the editor
11+
* @param {String} newContent - the DOM element's outerHTML after editing in live preview
12+
* @returns {String} - the updated content that should replace the original code in the editor
13+
*
14+
* NOTE: This function is a bit complex to read, read this jsdoc to understand the flow:
15+
*
16+
* First, we parse both the old and new content using DOMParser to get proper HTML DOM structures
17+
* Then we compare each element and text node between the old and new content
18+
*
19+
* the main goal is that we ONLY want to update text content, and not element nodes or their attributes
20+
* because if we allow element/attribute changes, the browser might try to fix the HTML
21+
* to make it syntactically correct or to make it efficient, which would mess up the user's original code
22+
* We don't want that - we need to respect how the user wrote their code
23+
* For example: if user wrote <div style="color: red blue green yellow"></div>
24+
* The browser sees this is invalid CSS and would remove the color attribute entirely
25+
* We want to keep that invalid code as it is because it's what the user wanted to do
26+
*
27+
* Here's how the comparison works:
28+
* - if both nodes are text: update the old text with the new text
29+
* - if both nodes are elements: we recursively check their children (for nested content)
30+
* - if old is text, new is element: replace text with element (like when user adds <br>)
31+
* - if old is element, new is text: replace element with text (like when user removes <br>)
32+
* note: when adding new elements (like <br> tags), we only copy the tag name and content,
33+
* never the attributes, to avoid internal Phoenix properties leaking into user's code
1034
*/
11-
function _findContentBoundaries(html) {
12-
const openTagEnd = html.indexOf(">") + 1;
13-
const closeTagStart = html.lastIndexOf("<");
14-
15-
if (openTagEnd > 0 && closeTagStart >= openTagEnd) {
16-
return {
17-
openTag: html.substring(0, openTagEnd),
18-
closeTag: html.substring(closeTagStart)
19-
};
35+
function _syncTextContentChanges(oldContent, newContent) {
36+
const parser = new DOMParser();
37+
const oldDoc = parser.parseFromString(oldContent, "text/html");
38+
const newDoc = parser.parseFromString(newContent, "text/html");
39+
40+
// as DOM parser will add the complete html structure with the HTML tags and all,
41+
// so we just need to get the main content
42+
const oldRoot = oldDoc.body;
43+
const newRoot = newDoc.body;
44+
45+
// here oldNode and newNode are full HTML elements which are direct children of the body tag
46+
function syncText(oldNode, newNode) {
47+
if (!oldNode || !newNode) {
48+
return;
49+
}
50+
51+
// if both the oldNode and newNode has text, replace the old node's text content with the new one
52+
if (oldNode.nodeType === Node.TEXT_NODE && newNode.nodeType === Node.TEXT_NODE) {
53+
oldNode.nodeValue = newNode.nodeValue;
54+
55+
} else if (
56+
// if both have element node, then we recursively get their child elements
57+
// this is so that we can get & update the text content in deeply nested DOM
58+
oldNode.nodeType === Node.ELEMENT_NODE &&
59+
newNode.nodeType === Node.ELEMENT_NODE
60+
) {
61+
62+
const oldChildren = oldNode.childNodes;
63+
const newChildren = newNode.childNodes;
64+
65+
const minLength = Math.min(oldChildren.length, newChildren.length);
66+
67+
for (let i = 0; i < minLength; i++) {
68+
syncText(oldChildren[i], newChildren[i]);
69+
}
70+
71+
// append if there are any new nodes, this is mainly when <br> tags needs to be inserted
72+
// as user pressed shift + enter to create empty lines in the new content
73+
for (let i = minLength; i < newChildren.length; i++) {
74+
const newChild = newChildren[i];
75+
let cleanChild;
76+
77+
if (newChild.nodeType === Node.ELEMENT_NODE) {
78+
// only the element name and not its attributes
79+
// this is to prevent internal properties like data-brackets-id, etc to appear in users code
80+
cleanChild = document.createElement(newChild.tagName);
81+
cleanChild.innerHTML = newChild.innerHTML;
82+
} else {
83+
// for text nodes, comment nodes, etc. clone normally
84+
cleanChild = newChild.cloneNode(true);
85+
}
86+
87+
oldNode.appendChild(cleanChild);
88+
}
89+
90+
// remove extra old nodes (maybe extra <br>'s were removed)
91+
for (let i = oldChildren.length - 1; i >= newChildren.length; i--) {
92+
oldNode.removeChild(oldChildren[i]);
93+
}
94+
95+
} else if (oldNode.nodeType === Node.TEXT_NODE && newNode.nodeType === Node.ELEMENT_NODE) {
96+
// when old has text node and new has element node
97+
// this generally happens when we remove the complete content which results in empty <br> tag
98+
// for ex: <div>hello</div>, here if we remove the 'hello' from live preview then result will be
99+
// <div><br></div>
100+
const replacement = document.createElement(newNode.tagName);
101+
replacement.innerHTML = newNode.innerHTML;
102+
oldNode.parentNode.replaceChild(replacement, oldNode);
103+
} else if (oldNode.nodeType === Node.ELEMENT_NODE && newNode.nodeType === Node.TEXT_NODE) {
104+
// this is opposite of previous one when earlier it was just <br> or some tag
105+
// and now we add text content in that
106+
const replacement = document.createTextNode(newNode.nodeValue);
107+
oldNode.parentNode.replaceChild(replacement, oldNode);
108+
}
109+
}
110+
111+
const oldEls = oldRoot.children;
112+
const newEls = newRoot.children;
113+
114+
for (let i = 0; i < Math.min(oldEls.length, newEls.length); i++) {
115+
syncText(oldEls[i], newEls[i]);
20116
}
21117

22-
return null;
118+
return oldRoot.innerHTML;
23119
}
24120

25121
/**
26122
* this function handles the text edit in the source code when user updates the text in the live preview
123+
*
27124
* @param {Object} message - the message object
28-
* {
29-
* livePreviewEditEnabled: true,
30-
element: the DOM element that was modified,
31-
oldContent: the text that was present before the edit,
32-
newContent: the new text,
33-
tagId: data-brackets-id of the DOM element,
34-
livePreviewTextEdit: true
35-
}
36-
*
37-
* The logic is: get the text in the editor using the tagId. split that text using the old content
38-
* join the text back and add the new content in between
39-
*/
125+
* - livePreviewEditEnabled: true
126+
* - livePreviewTextEdit: true
127+
* - element: element
128+
* - newContent: element.outerHTML (the edited content from live preview)
129+
* - tagId: Number (data-brackets-id of the edited element)
130+
* - isEditSuccessful: boolean (false when user pressed Escape to cancel, otherwise true always)
131+
*/
40132
function _editTextInSource(message) {
41133
const currLiveDoc = LiveDevMultiBrowser.getCurrentLiveDoc();
42134
if (!currLiveDoc || !currLiveDoc.editor || !message.tagId) {
43135
return;
44136
}
45137

46138
const editor = currLiveDoc.editor;
139+
47140
// get the start range from the getPositionFromTagId function
48141
// and we get the end range from the findMatchingTag function
49142
// NOTE: we cannot get the end range from getPositionFromTagId
@@ -62,20 +155,22 @@ define(function (require, exports, module) {
62155
const endPos = endRange.close.to;
63156

64157
const text = editor.getTextBetween(startPos, endPos);
65-
let splittedText;
66158

67-
// we need to find the content boundaries to find exactly where the content starts and where it ends
68-
const boundaries = _findContentBoundaries(text);
69-
if (boundaries) {
70-
splittedText = [boundaries.openTag, boundaries.closeTag];
71-
}
159+
// if the edit was cancelled (mainly by pressing Escape key)
160+
// we just replace the same text with itself
161+
// this is a quick trick because as the code is changed for that element in the file,
162+
// the live preview for that element gets refreshed and the changes are discarded in the live preview
163+
if(!message.isEditSuccessful) {
164+
editor.replaceRange(text, startPos, endPos);
165+
} else {
72166

73-
// if the text split was done successfully, apply the edit
74-
if (splittedText && splittedText.length === 2) {
75-
const finalText = splittedText[0] + message.newContent + splittedText[1];
167+
// if the edit operation was successful, we call a helper function that
168+
// is responsible to provide the actual content that needs to be written in the editor
169+
//
170+
// text: the actual current source code in the editor
171+
// message.newContent: the new content in the live preview after the edit operation
172+
const finalText = _syncTextContentChanges(text, message.newContent);
76173
editor.replaceRange(finalText, startPos, endPos);
77-
} else {
78-
console.error("Live preview text edit operation failed.");
79174
}
80175
}
81176

0 commit comments

Comments
 (0)