Open Bug 1331440 Opened 8 years ago Updated 2 years ago

stylo: Be smarter about generating change hints for appearing/disappearing pseudo-elements

Categories

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

defect

Tracking

()

Tracking Status
firefox57 --- wontfix

People

(Reporter: bholley, Unassigned)

References

(Blocks 1 open bug)

Details

Right now we generate ReconstructFrame on the originating element whenever a pseudo appears or disappears. This seems suboptimal in cases where the pseudo-element is NAC-backed, since we should just be able to handle it like an insertion (consider a ::before or ::after rule that matches on the root of a very deep tree).

Our current behavior is overly-conservative, so this doesn't affect correctness, only perf.

I'm not sure if Gecko does something smarter than what we're doing with stylo right now. If it doesn't, this is probably less of a priority to fix.
> Right now we generate ReconstructFrame on the originating element whenever a pseudo appears or disappears

Appears or disappears in what sense?  Due to changes in the stylesheet, or to the originating element's state?

What Gecko does with ::before and ::after specifically (which can only appear or disappear due to changes in the stylesheet) is reframe their parent.  See ElementRestyler::MaybeReframeForPseudo.

For ::first-line and ::first-letter Gecko just behaves incorrectly.  See bug 8253 (yes, four digits).
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #1)
> > Right now we generate ReconstructFrame on the originating element whenever a pseudo appears or disappears
> 
> Appears or disappears in what sense?  Due to changes in the stylesheet, or
> to the originating element's state?

I was thinking the latter, given that IIUC non-trivial stylesheet changes invalidate the entire rule tree.

> What Gecko does with ::before and ::after specifically (which can only
> appear or disappear due to changes in the stylesheet)

Is that really true? This would seem to be a counter-example: http://bit.ly/2jXtMnx

> is reframe their
> parent.  See ElementRestyler::MaybeReframeForPseudo.

Ok, so that suggests that what stylo is doing isn't worse than what gecko does. It would be good to optimize this though.

> For ::first-line and ::first-letter Gecko just behaves incorrectly.  See bug
> 8253 (yes, four digits).

Yikes. Is there any reason we don't just reframe the parent in that case too?
> I was thinking the latter

In that case, pseudo-elements generally cannot appear/disappear.  At least not the sort that would be represented by NAC.

> This would seem to be a counter-example: http://bit.ly/2jXtMnx

OK, we have a miscommunication.  "Changes in the stylesheet" == "any change that causes changes in which selectors match our element, for whatever reason" above.

> Ok, so that suggests that what stylo is doing isn't worse than what gecko does.

Yes.

> It would be good to optimize this though.

Sure, if it's easy. But I' really rather we didn't rathole on this.

> Is there any reason we don't just reframe the parent in that case too?

Reframing is easy.  But _detecting_ that we need to reframe (that is, that we should now have one of those and don't, or vice versa) quickly and without too many false positives is quite complicated in Gecko's setup.  In particular the "should have" conditions are fairly complex and not encoded anywhere central.
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #3)
> OK, we have a miscommunication.  "Changes in the stylesheet" == "any change
> that causes changes in which selectors match our element, for whatever
> reason" above.

Hm. Then what isn't a change in the stylesheet by that terminology? Changes in the properties inherited from the parent?
> Then what isn't a change in the stylesheet by that terminology?

Well, for example if we went from a checkbox to a text input without reframing to start with, that would cause ::placeholder to suddenly exist when it didn't before even though no selectors started or stopped matching.  Or if there were an attribute on number inputs to suppress the spinbuttons and we didn't reframe when it changed.  That sort of thing.
Priority: -- → P4
status-firefox57=wontfix unless someone thinks this bug should block 57
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.