Closed Bug 1358990 Opened 7 years ago Closed 7 years ago

stylo: Parse long ident return "A"

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: boris, Unassigned)

References

()

Details

I tried to parse a custom-ident in TransitionProperty::parse [1].

If the input is:
  transition-property: unsupported-property
try!(input.expect_ident()) returns: "unsupported-property"

If the input is: 
  transition-property: -other-unsupported-propert;
try!(input.expect_ident()) returns: "-other-unsupported-propert"

If the input is:
  transition-property: -other-unsupported-property;
try!(input.expect_ident()) returns: "A"

If the input is:
  transition-property: aaaaaaaaaaaaaaaaaaaaaaaaaaa;
try!(input.expect_ident()) returns: "A"

The returned value of the last two inputs are not correct.

[1] http://searchfox.org/mozilla-central/rev/313e5199bf58200f158c6fcbe193e41b88ed58a6/servo/components/style/properties/helpers/animated_properties.mako.rs#92-102
This is https://github.com/servo/rust-cssparser/issues/126 / https://github.com/servo/rust-cssparser/pull/127. I thought we had landed a fix already but it looks like we forgot :)
To be clear, input.expect_ident() returns the correct value, it's the wildcard binding what fails. You can work around it as:

let ident = try!(input.expect_ident());

match_ignore_ascii_case! { &ident,
   "foo" => ...,
   _ => TransitionProperty::Custom(ident),
}
(In reply to Emilio Cobos Álvarez [:emilio] from comment #2)
> To be clear, input.expect_ident() returns the correct value, it's the
> wildcard binding what fails. You can work around it as:
> 
> let ident = try!(input.expect_ident());
> 
> match_ignore_ascii_case! { &ident,
>    "foo" => ...,
>    _ => TransitionProperty::Custom(ident),
> }

Thanks! Let me work around this problem. :)
Yes, in fact the "least bad" fix is to remove support for bindings inside match_ignore_ascii_case, so you’d have to do that work-around :)

Don’t forget to use .to_ascii_lowercase() (or similar) if you do want to use a lower-cased string.
Fixed in:

https://github.com/servo/rust-cssparser/pull/140
https://github.com/servo/servo/pull/16589

The latter has yet to land, but let’s consider this fixed anyway.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
(In reply to Simon Sapin (:SimonSapin) from comment #5)
> Fixed in:
> 
> https://github.com/servo/rust-cssparser/pull/140
> https://github.com/servo/servo/pull/16589
> 
> The latter has yet to land, but let’s consider this fixed anyway.

Thanks! :)
You need to log in before you can comment on or make changes to this bug.