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

What to do with rgba(0 255 100%)? #562

Closed
chilts opened this issue May 5, 2015 · 4 comments
Closed

What to do with rgba(0 255 100%)? #562

chilts opened this issue May 5, 2015 · 4 comments
Labels
Milestone

Comments

@chilts
Copy link

chilts commented May 5, 2015

Hi @jakubpawlowicz,

It's me again (from http://cssminifier.com/). I've been digging through a number of files that get submitted to the site and trying to extract failures I see. This one is fairly simple I think, but the question is what to do with it.

Smallest example that fails:

body {
  background: rgba(0 255 100%);
}

The error is:

Error:TypeError: Cannot call method 'indexOf' of undefined

What's happening is that inside lib/selectors/optimizers/simple.js inside colorMininifier() the following regex is being matched /(rgb|rgba|hsl|hsla)\(([^\)]+)\)/g obviously. However, in the function the colorDef.split(',') only produces one tokens since there are no commas

The function then falls through to the tokens[1] (which is undefined) and therefore the .indexOf() fails with the above statement.

What's the best thing to do here? The rgba(0 255 100%) value is invalid (https://developer.mozilla.org/en/docs/Web/CSS/color_value#rgba%28%29) since it mixes integer and percentage notation. Do we leave it alone and output a warning? Do we try and do something with it, do we just omit it completely? Am asking these questions since I'm sure you've already got procedures for doing something when processing invalid input.

Oh, and I'm happy to create a PR for it too so I can get into it a little bit. :) Cheers.

@jakubpawlowicz
Copy link
Collaborator

Hi @chilts,

Thanks for coming back :-)

I think there are 2 questions:

  • whether rgb/rgba without commas are even valid (I haven't checked yet)
  • if so, in which browsers, so we change the logic to accommodate such rgb declarations

Good idea with analysing the common errors though 💯

@chilts
Copy link
Author

chilts commented May 11, 2015

From what I can see, the rgb() and rgba() must enclose 3 or 4 integer or percent values. Anything else (including mixing them) is invalid. The example above is therefore wrong in at least three ways:

  1. the values mix both integer and percent values
  2. the values are not separated by a comma
  3. it is rgba but only has three values, not four

I'm afraid I don't have access to many browsers, just Firefox and Chromium on Linux so can't test across browsers/platforms. Any info you can find is much appreciated.

@jakubpawlowicz jakubpawlowicz added this to the 3.3 milestone May 20, 2015
@jakubpawlowicz
Copy link
Collaborator

Well I think we should remove the declarations as browsers will ignore them anyways. Let's see if it's easy to do so, otherwise we can also leave them as is.

@jakubpawlowicz
Copy link
Collaborator

So it will stay as is for now as it's too tricky to remove these declarations effectively.

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

No branches or pull requests

2 participants