1375971 - stylo: Shouldn't compute the actual ComputedValues for ::first-letter during servo traversal

Status: NEW

(Core :: CSS Parsing and Computation, enhancement, P4)

Description

[Boris Zbarsky [:bzbarsky]]

::first-letter inherits from the parent of the text it's wrapping, not from the block.  So the styles we compute for it may well be wrong, unless the text is directly in the block.

Emilio suggested having the eager pseudos be something like

  Option<Either<ComputedValues, RuleNode>>

and then using ComputedValues for before/after/first-line and RuleNode for first-letter.  We can use the RuleNode being there or not to determine whether to reframe, and ideally also to avoid redoing the selector matching from post-traversal when we finally go to figure out the right style for the first-letter.

Comment 1

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) from comment #0)
> ::first-letter inherits from the parent of the text it's wrapping, not from
> the block.  So the styles we compute for it may well be wrong, unless the
> text is directly in the block.
> 
> Emilio suggested having the eager pseudos be something like
> 
>   Option<Either<ComputedValues, RuleNode>>

Note that this particular layout will bloat each entry from 1 word to 3 (because we don't pack enums, and because the NonZero optimization doesn't extend into neseted enums).

Given that these data structures are persistent, and given that we have a small number of entries in known, fixed slots, it seems better just to store the values as explicit fields and then add some iterator APIs on the wrapper type to make it nice.

Note also that I'm messing around with these data structures a tiny bit in bug 1371049, but that's minor and unlikely to conflict here.

Comment 2

[Bobby Holley (:bholley)]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #1)
> (In reply to Boris Zbarsky [:bz] (if a patch has no decent message,
> automatic r-) from comment #0)
> > ::first-letter inherits from the parent of the text it's wrapping, not from
> > the block.  So the styles we compute for it may well be wrong, unless the
> > text is directly in the block.
> > 
> > Emilio suggested having the eager pseudos be something like
> > 
> >   Option<Either<ComputedValues, RuleNode>>
> 
> Note that this particular layout will bloat each entry from 1 word to 3
> (because we don't pack enums, and because the NonZero optimization doesn't
> extend into neseted enums).
> 
> Given that these data structures are persistent, and given that we have a
> small number of entries in known, fixed slots, it seems better just to store
> the values as explicit fields and then add some iterator APIs on the wrapper
> type to make it nice.
> 
> Note also that I'm messing around with these data structures a tiny bit in
> bug 1371049, but that's minor and unlikely to conflict here.

(that said, it would still probably save you time to cherry-pick those patches and work on top of them).

Comment 3

[Boris Zbarsky [:bzbarsky]]

This isn't urgent or blocking me, so it can just wait.

Comment 4

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals