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

Rgb color with percent parsed incorrectly #1736

Closed
HonzaCustomInk opened this issue Nov 14, 2019 · 1 comment · Fixed by #1742
Closed

Rgb color with percent parsed incorrectly #1736

HonzaCustomInk opened this issue Nov 14, 2019 · 1 comment · Fixed by #1742

Comments

@HonzaCustomInk
Copy link

HonzaCustomInk commented Nov 14, 2019

Description/Steps to reproduce

Color incorrectly parses colors given with percentages. This manifest itself at least when SVGs with such colors are being parsed, but perhaps in other scenarios as well

Link to reproduction test-case

a JS minimal working example:

var paper = require('paper-jsdom');

red = new paper.Color('rgb(100%,0%,0%)')
console.log(red.toString())

which prints

{ red: 0.39216, green: 0, blue: 0 }

Expected result

{ red: 1, green: 0, blue: 0 }

Additional information

I suspect the problem is in the code that parses the colors, which seems to assume rgb always has values between 0 and 255, see here (although the hsl code seems to be able to work with % just fine. Indeed, rgb(255%,0%,0%) produces { red: 1, green: 0, blue: 0 }. Also, there's no test case for this syntax.

That this is a valid way of specifying color in SVGs can be seen in the docs, see the example in section 3.1 and the syntax in section 9. Additionally, I've encountered this on real-life SVG data.

It also seems to be a valid CSS color spec. Search for "RGB syntax variations" or Functional Notation.

MacOS 10.14.6, Node v12.11.1

PS: I'll work around this for now by pre-processing my files. Also, I am not a JS person and do not have the project set up; but if there's enough will and interaction, I might dedicate some time to setting up the project and to fixing this myself.

@sapics
Copy link
Member

sapics commented Dec 6, 2019

@HonzaRepresent Thanks for a bug report!
I made a PR to fix this issue, sorry I didn't read the PS part at that time.
If you have any updates, please teach us.

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

Successfully merging a pull request may close this issue.

3 participants