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

preserve vendor prefixes #27

Closed
ralyodio opened this issue Feb 12, 2013 · 19 comments · Fixed by #220
Closed

preserve vendor prefixes #27

ralyodio opened this issue Feb 12, 2013 · 19 comments · Fixed by #220

Comments

@ralyodio
Copy link

Is it possible to preserve vendor prefixes? It seems all of my gradients are stripped out, down to just linear-gradient() and filter:..

I'm using the gradient generator here: http://www.colorzilla.com/gradient-editor/
I need to preserve -webkit, etc.

This:

background: #ffffff; background: -moz-linear-gradient(top,  #ffffff 0%, #e5e5e5 100%); background: -webkit-gradient(linear, left top, left bottom, color-stop(0%,#ffffff), color-stop(100%,#e5e5e5)); background: -webkit-linear-gradient(top,  #ffffff 0%,#e5e5e5 100%); background: -o-linear-gradient(top,  #ffffff 0%,#e5e5e5 100%); background: -ms-linear-gradient(top,  #ffffff 0%,#e5e5e5 100%); background: linear-gradient(to bottom,  #ffffff 0%,#e5e5e5 100%); filter: progid:DXImageTransform.Microsoft.gradient( startColorstr='#ffffff', endColorstr='#e5e5e5',GradientType=0 ); ```

is stripped down to this:

background: linear-gradient(to bottom,  #ffffff 0%,#e5e5e5 100%); filter: progid:DXImageTransform.Microsoft.gradient( startColorstr=#ffffff, endColorstr=#e5e5e5,GradientType=0 );
@andrewrk
Copy link
Contributor

Seems like something we should do. A PR would be welcome.

@ralyodio
Copy link
Author

What is a PR? --nm, Pull Request I think you mean.

I found this bug here, and it looks like its not resolved...

NV/CSSOM#16

@andrewrk
Copy link
Contributor

Ah. Unfortunate that he hasn't fixed the bug in 2 years.

@ralyodio
Copy link
Author

As a work around, is there anyway of preserving existing style attributes? I can move all my rules into the style attribute, but when I do that, they also get wiped out. Or maybe a way to add data-ignore to an anchor tag for example to skip processing for that element.

@andrewrk
Copy link
Contributor

Yes. in the options object, set removeStyleTags and removeLinkTags to false.

@ralyodio
Copy link
Author

This looks like it removes style tags, not style attribute on an existing element.

@andrewrk
Copy link
Contributor

Ah. Sorry I did not realize that is what you meant. I do not know of a workaround for your issue.

@ralyodio
Copy link
Author

I'd be happy to write the code for it, just not sure where it would go. I think the easiest thing would be to check for data-ignore and skip the processing if this attribute is present on a node. If you can point me in the right direction, I'll work on a patch.

<a href="#" style="background: -moz-gradient(); background: -webkit-gradient(); etc etc" data-ignore>foobar</a>

@andrewrk
Copy link
Contributor

Unfortunately I'm not so familiar with a lot of the core juice code myself; I've just been doing some maintenance on the module. I cannot provide that guidance. However I do think that if there is a patch to address this, we don't need to add data-ignore - we should solve the problem by not stripping out existing styles.

@andrewrk
Copy link
Contributor

You could probably relatively easily patch the code to append the styles instead of replace.

@ralyodio
Copy link
Author

I think the problem is that the module basically identifies all css on the page and then builds a cssom tree of it, regardless of whether it was inline or in a rule. Ignoring inline styles would probably cause a lot of problems for people. I am just proposing a way to ignore that processing entirely in the event of a data-ignore-cssom attribute.

@jeremypeter
Copy link

Here's a quick fix in the meantime I made which uses the "data-ignore" attribute on your style tags because I needed a way to preserve my media queries. jeremypeter@f1d8420
Code should look like:

<style type="text/css" data-ignore="ignore" >

Styles to ignore here

</style> 

@stevenvachon
Copy link

use autoprefixer

@bago
Copy link
Contributor

bago commented Jun 23, 2016

Fixing this issue requires 2 major changes:

  1. Move away from CSSOM and use a parser that actually gives us multiple occourences of the same property being declared multiple times with multiple value)
  2. Alter Juice behaviour so to be able to keep multiple occourences of the same property: this is controversial because many times it does inline a lot of "useless" stuff, but I like "styliner" behaviour that does inline multiple occourences only if they are declared in the same "rule".

The first issue is easily fixed by #215 , now I'm working on the second one.

.selector {
  background: #ffffff;
  background: -moz-linear-gradient(top,  #ffffff 0%, #e5e5e5 100%);
  background: linear-gradient(to bottom,  #ffffff 0%,#e5e5e5 100%);
}

Would correclty inline the 3 background properties while

.selector {
  background: #ffffff;
  background: -moz-linear-gradient(top,  #ffffff 0%, #e5e5e5 100%);
}
.selector {
  background: linear-gradient(to bottom,  #ffffff 0%,#e5e5e5 100%);
}

Would only inline the last background, because it is on a separate rule.

Does this make sense to you?
i

@bago
Copy link
Contributor

bago commented Jun 23, 2016

Sorry if I confused you, but my branch is an "enabler" to fix this, but still doesn't fix this.

So this one should be kept opened... please tell me if the "keep multiple property values only if they are declared in the same rule", makes sense to you (@jrit , @chovy and other participants)

@jrit
Copy link
Collaborator

jrit commented Jun 23, 2016

github auto-closed this one, reopening

@jrit jrit reopened this Jun 23, 2016
@stevenvachon
Copy link

You should probably minify the css before attempting to work with it. Many things will be optimized, such as adjacent rules.

@bago
Copy link
Contributor

bago commented Jun 24, 2016

@stevenvanchon: merging adjacent rules will break prevent "user decision" about what to inline. So this is not a good idea. THe idea is that we BY PURPOSE decide to inline multiple declarations for the same property ONLY when the declarations are in the same rule. We do this because we EXPECT people to use a single rule when they do similar "tricks". If we wanted to use a different rule (like "SAME SELECTOR" instead of "SAME RULE") we would have implemented it that way.

So I don't think that "minify before inline" is a good suggestion for the general audience.

I already implemented this "behaviour" and I'll commit and PR very soon.

@stevenvachon
Copy link

Interesting -- thanks.

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

Successfully merging a pull request may close this issue.

6 participants