Skip to content

Commit

Permalink
Support floating point hue in hsl() and hsla() (#31)
Browse files Browse the repository at this point in the history
Browsers support a color like `hsl(120.152, 100%, 75%)` but this library
would incorrectly parse the hue and fallback to black.

Changes parsing the hue as an integer to parsing as a number.

[CSS Color Module Level 3
4.2.4](https://www.w3.org/TR/css-color-3/#hsl-color) suggests that the
hue should be a number (and not strictly an integer):

> Hue is represented as an angle of the color circle (i.e. the rainbow
represented in a circle). This angle is so typically measured in degrees
that the unit is implicit in CSS; syntactically, only a \<number\> is
given.

CSS Color Module Level 4 is more explicit that hue [can be a
number](https://www.w3.org/TR/css-color/#typedef-hue).

This PR also comments out the `parse_integer` and `parse_list_integer`
functions (and relevant tests) from `src/stream.rs` because leaving them
in the code produced these warnings during build:

```
warning: methods `parse_integer` and `parse_list_integer` are never used
   --> src/stream.rs:417:12
    |
109 | impl<'a> Stream<'a> {
    | ------------------- methods in this implementation
...
417 |     pub fn parse_integer(&mut self) -> Result<i32, Error> {
    |            ^^^^^^^^^^^^^
...
447 |     pub fn parse_list_integer(&mut self) -> Result<i32, Error> {
    |            ^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default
```

Happy to uncomment or remove them completely if that's preferred.

Related to vercel/satori#602
  • Loading branch information
erxclau authored Dec 3, 2024
1 parent 6c3a1d0 commit 8fdef18
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 70 deletions.
16 changes: 14 additions & 2 deletions src/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ impl Stream<'_> {
} else if name == "hsl" || name == "hsla" {
self.consume_byte(b'(')?;

let mut hue = self.parse_list_integer()?;
hue = ((hue % 360) + 360) % 360;
let mut hue = self.parse_list_number()?;
hue = ((hue % 360.0) + 360.0) % 360.0;

let saturation = f64_bound(0.0, self.parse_list_number_or_percent()?, 1.0);
let lightness = f64_bound(0.0, self.parse_list_number_or_percent()?, 1.0);
Expand Down Expand Up @@ -554,6 +554,18 @@ mod tests {
Color::new_rgba(127, 255, 127, 127)
);

test!(
hsl_with_hue_float,
"hsl(120.152, 100%, 75%)",
Color::new_rgba(127, 255, 127, 255)
);

test!(
hsla_with_hue_float,
"hsla(120.152, 100%, 75%, 0.5)",
Color::new_rgba(127, 255, 127, 127)
);

macro_rules! test_err {
($name:ident, $text:expr, $err:expr) => {
#[test]
Expand Down
68 changes: 0 additions & 68 deletions src/stream.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright 2018 the SVG Types Authors
// SPDX-License-Identifier: Apache-2.0 OR MIT

use std::str::FromStr;

use crate::Error;

/// Extension methods for XML-subset only operations.
Expand Down Expand Up @@ -409,52 +407,6 @@ impl<'a> Stream<'a> {
&self.text[self.pos..]
}

/// Parses integer number from the stream.
///
/// Same as [`parse_number()`], but only for integer. Does not refer to any SVG type.
///
/// [`parse_number()`]: #method.parse_number
pub fn parse_integer(&mut self) -> Result<i32, Error> {
self.skip_spaces();

if self.at_end() {
return Err(Error::InvalidNumber(self.calc_char_pos()));
}

let start = self.pos();

// Consume sign.
if self.curr_byte()?.is_sign() {
self.advance(1);
}

// The current char must be a digit.
if !self.curr_byte()?.is_digit() {
return Err(Error::InvalidNumber(self.calc_char_pos_at(start)));
}

self.skip_digits();

// Use the default i32 parser now.
let s = self.slice_back(start);
match i32::from_str(s) {
Ok(n) => Ok(n),
Err(_) => Err(Error::InvalidNumber(self.calc_char_pos_at(start))),
}
}

/// Parses integer from a list of numbers.
pub fn parse_list_integer(&mut self) -> Result<i32, Error> {
if self.at_end() {
return Err(Error::UnexpectedEndOfStream);
}

let n = self.parse_integer()?;
self.skip_spaces();
self.parse_list_separator();
Ok(n)
}

/// Parses number or percent from the stream.
///
/// Percent value will be normalized.
Expand Down Expand Up @@ -494,23 +446,3 @@ impl<'a> Stream<'a> {
}
}
}

#[rustfmt::skip]
#[cfg(test)]
mod tests {
use super::*;

#[test]
fn parse_integer_1() {
let mut s = Stream::from("10");
assert_eq!(s.parse_integer().unwrap(), 10);
}

#[test]
fn parse_err_integer_1() {
// error because of overflow
let mut s = Stream::from("10000000000000");
assert_eq!(s.parse_integer().unwrap_err().to_string(),
"invalid number at position 1");
}
}

0 comments on commit 8fdef18

Please sign in to comment.