Closed Bug 1457332 Opened 2 years ago Closed 2 years ago

Derive ToCss for values::generics::counters::Counters

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: xidorn, Assigned: emilio)

References

Details

Attachments

(1 file)

From bug 1434130 comment 67:

(In reply to Emilio Cobos Álvarez [:emilio] (Away-ish from 27/04 to 09/05) from comment #67)
> ::: servo/components/style/values/generics/counters.rs:62
> (Diff revision 1)
> >  /// A generic value for lists of counters.
> >  ///
> >  /// Keyword `none` is represented by an empty vector.
> >  #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo,
> >           ToComputedValue)]
> > -pub struct Counters<I>(Box<[(CustomIdent, I)]>);
> > +pub struct Counters<I>(#[css(if_empty = "none")] Box<[(CustomIdent, I)]>);
> 
> derive ToCss while at it, could do with `iterable`?
Assignee: nobody → emilio
Comment on attachment 8971847 [details]
Bug 1457332: Derive ToCss for Counters.

https://reviewboard.mozilla.org/r/240600/#review246284

::: servo/components/style/values/generics/counters.rs:17
(Diff revision 1)
> +         ToComputedValue, ToCss)]
> +pub struct CounterPair<Integer> {
> +    /// The name of the counter.
> +    pub name: CustomIdent,
> +    /// The value of the counter / increment / etc.
> +    pub delta: Integer,

I think naming it `delta` is misleading, since it is a "delta" only on one of the three cases... Maybe just name it `value` and comment exactly that what it means when used on different properties.

::: servo/components/style/values/specified/counters.rs:69
(Diff revision 1)
>              Ok(&Token::Ident(ref ident)) => CustomIdent::from_ident(location, ident, &["none"])?,
>              Ok(t) => return Err(location.new_unexpected_token_error(t.clone())),
>              Err(_) => break,
>          };
>  
> -        let counter_delta = input
> +        let delta = input

So this is a pre-existing misleading name... let's fix it as well :)
Attachment #8971847 - Flags: review?(xidorn+moz) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/54a4ed5b619b
Derive ToCss for Counters. r=xidorn
https://hg.mozilla.org/mozilla-central/rev/54a4ed5b619b
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.