grapesjs icon indicating copy to clipboard operation
grapesjs copied to clipboard

Inline CSS Comments break Code Manager

Open emilsedgh opened this issue 6 years ago • 6 comments

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:

screenshot_20181113_165603

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.

emilsedgh avatar Nov 14 '18 01:11 emilsedgh

Hi Emil, I tried this https://jsfiddle.net/37n5691f/ but can't see the issue

artf avatar Nov 16 '18 00:11 artf

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.

no-response[bot] avatar Nov 26 '18 00:11 no-response[bot]

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.

emilsedgh avatar Aug 24 '19 11:08 emilsedgh

Thanks @emilsedgh Ok, I think we have to improve the style attribute parser here

Suggestions are welcome

artf avatar Aug 26 '19 22:08 artf

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.

emilsedgh avatar Aug 27 '19 01:08 emilsedgh

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

artf avatar Sep 01 '19 16:09 artf