1358990 - stylo: Parse long ident return "A"

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Boris Chiou [:boris]]

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

Comment 1

[Simon Sapin (:SimonSapin)]

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 :)

Comment 2

[Emilio Cobos Álvarez (:emilio)]

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),
}

Comment 3

[Boris Chiou [:boris]]

(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. :)

Comment 4

[Simon Sapin (:SimonSapin)]

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.

Comment 5

[Simon Sapin (:SimonSapin)]

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.

Comment 6

[Boris Chiou [:boris]]

(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! :)