grapesjs
grapesjs copied to clipboard
Inline CSS Comments break Code Manager
Apparently this is valid piece of html and css:
<div style="color: red; /* height 200px; */; font-weight: bold;">
(Hence the commented out height property)
Grapesjs has trouble dealing with such attribute as the style attribute on the model is stored as this:
The big problem is that CssGenerator's buildFromObject
considers /*
as part of the rule name for /* height
, and concatenates it the whole css string. So the css code is basically commented out when it reaches this model and is rendered unusable blob of commented out css.
Obviously the real fix is not on CodeManager. Whoever parses /* height
to be a rule name is making a mistake.
Hi Emil, I tried this https://jsfiddle.net/37n5691f/ but can't see the issue
This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.
Alright I finally managed to reproduce this again. Reopening with better fiddle.
The code inside gjs is
<p style="/* color: #ffffff; */"></p>
<span style="color: red;">Bar</span>
The CSS retrieved by editor.getCss()
is:
* { box-sizing: border-box; } body {margin: 0;}.c620{/* color:#ffffff;}.c629{color:red;}
The bug is that the /* color:#ffffff;
doesn't have the closing comment tag. Therefore, it'd render any css that comes after it.
My apologies for the late response.
Thanks @emilsedgh Ok, I think we have to improve the style attribute parser here
Suggestions are welcome
How about a regex to remove all comments, and then continue parsing as usual?
I would be a bit worried of corner cases the regexp might fail and also the performance impact, but that'd be the easiest way to solve it.
It's interesting that browsers implement a DOMParser but provide no reliable way of parsing styles.
I would be a bit worried of corner cases the regexp might fail and also the performance impact, but that'd be the easiest way to solve it.
Yeah, regexp might be the "easiest", but the performance impact is also one of my concerns. I was thinking about something, IMHO, simpler.
while (str.indexOf('/*') >= 0) {
const start = str.indexOf('/*');
const end = str.indexOf('*/') + 2;
str = str.replace(str.slice(start, end), '');
}
It might be a case to setup a test case with https://jsperf.com