Closed Bug 1369187 Opened 3 years ago Closed 3 years ago

stylo: Implement the basic building blocks for ::first-line and ::first-letter.

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(5 files)

So Bobby recently asked for this, and I had this patch lying around.

I guess it's better to land them in this state so Boris and I can work on the remaining pieces in parallel.

This is just the building blocks, so there's some (a lot) of stuff that's missing. In particular:

 * The "only some properties affect first-line/first-letter" bits.
 * The restyle handling for them is terribly inefficient (we just reframe, which is unacceptable for shipping). More on this below.
 * The "this element will never generate a first-line/first-letter pseudo" is also not done (the API is there though).

I think it's worth landing, because there are a few open questions re. the rest. In particular:

# CalcStyleDifference

Right now for ::before and ::after we find the nsStyleContext of the relevant frame on the fly, which is ok. It's not clear to me though whether we want to do frame tree traversal in order to get the ::first-line/::first-letter pseudo-element.

In particular, given we now diff all the structs unconditionally, and the property set that applies to them is reduced, the win of the "has this struct been used? If not, avoid adding change hints due to it" may not be as huge.

So my proposal for it is allowing to call CalcStyleDifference on a pair of ComputedValues. We're going to need something like that for ::visited anyway.

# Frame traversal

I expect Boris to be able to just answer/implement most of the bits related to this. What is a good API we could expose to avoid adding a perf hit for normal restyles, while making these correct?

Thoughts on those?
(In reply to Emilio Cobos Álvarez [:emilio] from comment #0)
> So my proposal for it is allowing to call CalcStyleDifference on a pair of
> ComputedValues. We're going to need something like that for ::visited anyway.

Yeah, for :visited, we could either compute the nsStyleContext for visited and then compare those like Gecko does[1], or we could directly compare the ServoComputedValues like you are suggesting.  (For :visited, we may want to take the same optimization as Gecko and only compare visited-dependent properties.)

[1]: http://searchfox.org/mozilla-central/rev/1a0d9545b9805f50a70de703a3c04fc0d22e3839/layout/style/nsStyleContext.cpp#1146-1201
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #7)
> (In reply to Emilio Cobos Álvarez [:emilio] from comment #0)
> > So my proposal for it is allowing to call CalcStyleDifference on a pair of
> > ComputedValues. We're going to need something like that for ::visited anyway.
> 
> Yeah, for :visited, we could either compute the nsStyleContext for visited
> and then compare those like Gecko does[1], or we could directly compare the
> ServoComputedValues like you are suggesting.  (For :visited, we may want to
> take the same optimization as Gecko and only compare visited-dependent
> properties.)

Right, that's also true to some extent for ::first-letter and ::first-line, so that's why I plan to share the infrastructure here with that :)

I think we should try to not compute the nsStyleContext for visited, since it's only used in a few places, and nsStyleContext's are quite fat right now, so they take up a bit of memory.
Comment on attachment 8873208 [details]
Bug 1369187: Don't return an old style context for ::first-line and ::first-letter, but not crash either.

https://reviewboard.mozilla.org/r/144656/#review148888
Attachment #8873208 - Flags: review?(bobbyholley) → review+
Comment on attachment 8873209 [details]
Bug 1369187: style: Add an API to fast-reject eager pseudos.

https://reviewboard.mozilla.org/r/144658/#review148890
Attachment #8873209 - Flags: review?(bobbyholley) → review+
Comment on attachment 8873210 [details]
Bug 1369187: style: Add first-line and first-letter to the set of eager pseudo styles.

https://reviewboard.mozilla.org/r/144660/#review148892
Attachment #8873210 - Flags: review?(bobbyholley) → review+
Comment on attachment 8873211 [details]
Bug 1369187: style: Use ArrayVec for the pseudo-elements we need to restyle.

https://reviewboard.mozilla.org/r/144662/#review148894
Attachment #8873211 - Flags: review?(bobbyholley) → review+
Comment on attachment 8873212 [details]
Bug 1369187: style: Assert that if we find an eager, element-backed pseudo, it's ::before or ::after.

https://reviewboard.mozilla.org/r/144664/#review148896
Attachment #8873212 - Flags: review?(bobbyholley) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/cf869515f9be
Don't return an old style context for ::first-line and ::first-letter, but not crash either. r=bholley
https://hg.mozilla.org/integration/autoland/rev/2fca98f65d11
Update expectations. r=emilio
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/fec3a4b50acd
More expectation updates. r=emilio
You need to log in before you can comment on or make changes to this bug.