Closed Bug 1466645 Opened Last year Closed Last year

PropertyId::name & users could be nicer.

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(3 files)

We only use it in error paths.
Just realised that serialize_identifier for custom properties doesn't work, and produces a wrong result with properties like `--0`.

Will fix, though it requires cssparser changes.
Depends on: 1466789
Comment on attachment 8983161 [details]
Bug 1466645: Avoid useless allocations in custom property name serialization.

https://reviewboard.mozilla.org/r/249016/#review255686
Attachment #8983161 - Flags: review?(xidorn+moz) → review+
Comment on attachment 8983162 [details]
Bug 1466645: Remove PropertyId::name.

https://reviewboard.mozilla.org/r/249018/#review255688

::: servo/components/style_traits/lib.rs:180
(Diff revision 2)
> -    pub fn new_invalid(name: CowRcStr<'i>, value_error: ParseError<'i>) -> ParseError<'i> {
> +    pub fn new_invalid<S>(name: S, value_error: ParseError<'i>) -> ParseError<'i>
> +    where
> +        S: Into<CowRcStr<'i>>,

Maybe you can just have `new_invalid` accept a `&PropertyId` instead, which should simplify your code above.
Attachment #8983162 - Flags: review?(xidorn+moz) → review+
Comment on attachment 8983163 [details]
Bug 1466645: Make getting a property name explicitly an indexing operation.

https://reviewboard.mozilla.org/r/249020/#review255692
Attachment #8983163 - Flags: review?(xidorn+moz) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bfc3d86b042
Avoid useless allocations in custom property name serialization. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f5c7353cfa4
Make getting a property name explicitly an indexing operation. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/521224a55cbe
Remove PropertyId::name. r=xidorn
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #9)
> Comment on attachment 8983162 [details]
> Bug 1466645: Remove PropertyId::name.
> 
> https://reviewboard.mozilla.org/r/249018/#review255688
> 
> ::: servo/components/style_traits/lib.rs:180
> (Diff revision 2)
> > -    pub fn new_invalid(name: CowRcStr<'i>, value_error: ParseError<'i>) -> ParseError<'i> {
> > +    pub fn new_invalid<S>(name: S, value_error: ParseError<'i>) -> ParseError<'i>
> > +    where
> > +        S: Into<CowRcStr<'i>>,
> 
> Maybe you can just have `new_invalid` accept a `&PropertyId` instead, which
> should simplify your code above.

I cannot because it lives in style_traits.
You need to log in before you can comment on or make changes to this bug.