Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline CSS Comments break Code Manager #1577

Closed
emilsedgh opened this issue Nov 14, 2018 · 6 comments
Closed

Inline CSS Comments break Code Manager #1577

emilsedgh opened this issue Nov 14, 2018 · 6 comments

Comments

@emilsedgh
Copy link
Contributor

emilsedgh commented Nov 14, 2018

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.

@artf
Copy link
Member

artf commented Nov 16, 2018

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

@no-response
Copy link

no-response bot commented Nov 26, 2018

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 no-response bot closed this as completed Nov 26, 2018
@emilsedgh
Copy link
Contributor Author

emilsedgh commented Aug 24, 2019

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.

@artf
Copy link
Member

artf commented Aug 26, 2019

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

Suggestions are welcome

@emilsedgh
Copy link
Contributor Author

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.

@artf
Copy link
Member

artf commented Sep 1, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants