In bug 1352565 comment 6 bz provided the following testcase: <style> input::placeholder { color: purple; } input.clicked::placeholder { color: yellow } </style> <input placeholder="Text" onclick="this.className = 'clicked'"> In Stylo right now, we'll end up in the restyle hint machinery for the attribute change, and presumably generate eRestyle_Self, since there's no combinator on the right. We probably need to detect if there's a pseudo-element and: (a) In the NAC-back case, use eRestyle_Subtree, which will cause us to redo selector matching for the NAC, and (b) In the non-NAC-backed case, generate a change hint that tells the post-traversal to re-resolve pseudo styles on the frames that own them. Thoughts? See also the discussion around ::first-letter in bug 1324618.

My idea for this was roughly the following: * Do the cascade for the element normally, also matching against selectors with the pseudos, but... * Don't compute style for those pseudos just yet, just note in a bitflag they're affected by these styles, and store those flags in the "main" ComputedValues of the element (or ElementStyles). My idea is that this is fine because the # of rules with pseudos is usually much lower than the rest of rules in the page. This allows us to whether we're in any of these cases: 1) Rules matched, but no longer match. 2) Rules didn't match, but now do. 3) Rules matched and still match. We can also know whether the pseudo style is actually effective, if when we resolve it for the first time we keep it cached around with the element. This allows us to compute style difference in 3), and handle the changes as we see fit. Servo does this to avoid computing custom pseudos multiple times, and stores it in a HashMap. Probably a SmallVec/AutoTArray is enough. With that information, we can handle pretty much every case. We may also need to handle the case of "we've actually computed the style, and turns out to be useless" (i.e., if we don't create a frame for a placeholder or something like that). We could have a function to uncache the pseudos, or have logic to never cache it, etc. Does that sound reasonable?

Nice idea! So to clarify, the plan would be to have selector matching fill a PseudoBitFlags outparam with the pseudos for which the given element is the originating element, and then we'd store those flags on the ElementData? If the flags change, we reframe (though maybe we can do better). If the flags don't change, but we _did_ rematch selectors, we'd need some sort of change hint to tell the post-traversal that it needs to rematch selectors for the pseudos, or an eRestyle_NACDescendants restyle hint in the case of NAC-backed content (basically the setup proposed in comment 0, though it probably doesn't need to be a part of restyle hint generation as long as restyle hints include selectors that terminate in pseudo-elements, which I think they do). Would we still need eagerly-cascaded pseudos? I think we would, at least for ::before/::after, because we need to do the cascade to check for the degenerate case (bug 1352565). And it's not wasted work, because most elements that match ::before/::after will indeed need those styles. Are there any others? Assuming we still do need eagerly-cascaded pseudos, then we still want the patches in bug 1335708 and bug 1352565, I think. I don't think we need to cache any other pseudo styles on the ElementData, since the post-traveral can find them on the frame. Emilio, does this all sound like what you had in mind? If so we can see what bz thinks.

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #2) > So to clarify, the plan would be to have selector matching fill a > PseudoBitFlags outparam with the pseudos for which the given element is the > originating element, and then we'd store those flags on the ElementData? If > the flags change, we reframe (though maybe we can do better). If the flags > don't change, but we _did_ rematch selectors, we'd need some sort of change > hint to tell the post-traversal that it needs to rematch selectors for the > pseudos, or an eRestyle_NACDescendants restyle hint in the case of > NAC-backed content (basically the setup proposed in comment 0, though it > probably doesn't need to be a part of restyle hint generation as long as > restyle hints include selectors that terminate in pseudo-elements, which I > think they do). Right. > Would we still need eagerly-cascaded pseudos? I think we would, at least for > ::before/::after, because we need to do the cascade to check for the > degenerate case (bug 1352565). And it's not wasted work, because most > elements that match ::before/::after will indeed need those styles. Are > there any others? Yes, I believe it still makes sense to do the ::before and ::after cascade during the traversal. > Assuming we still do need eagerly-cascaded pseudos, then we still want the > patches in bug 1335708 and bug 1352565, I think. > > I don't think we need to cache any other pseudo styles on the ElementData, > since the post-traveral can find them on the frame. Right, the thing that caching allows to do is _efficiently_ knowing if there's a frame at all. If you keep them cached when we use them, knowing "is there a first-line frame" around is efficient. Otherwise you need to guess whether you created a first-line frame at all (which may not be that hard actually), but you need to walk the frame tree. > Emilio, does this all sound like what you had in mind? If so we can see what > bz thinks. Yes, feedback from boris would be appreciated. Overall because he tends to find flaws in the different approaches quite fast ;)

(In reply to Emilio Cobos Álvarez [:emilio] from comment #3) > (In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #2) > > So to clarify, the plan would be to have selector matching fill a > > PseudoBitFlags outparam with the pseudos for which the given element is the > > originating element, and then we'd store those flags on the ElementData? If > > the flags change, we reframe (though maybe we can do better). If the flags > > don't change, but we _did_ rematch selectors, we'd need some sort of change > > hint to tell the post-traversal that it needs to rematch selectors for the > > pseudos, or an eRestyle_NACDescendants restyle hint in the case of > > NAC-backed content (basically the setup proposed in comment 0, though it > > probably doesn't need to be a part of restyle hint generation as long as > > restyle hints include selectors that terminate in pseudo-elements, which I > > think they do). > > Right. > > > Would we still need eagerly-cascaded pseudos? I think we would, at least for > > ::before/::after, because we need to do the cascade to check for the > > degenerate case (bug 1352565). And it's not wasted work, because most > > elements that match ::before/::after will indeed need those styles. Are > > there any others? > > Yes, I believe it still makes sense to do the ::before and ::after cascade > during the traversal. > > > Assuming we still do need eagerly-cascaded pseudos, then we still want the > > patches in bug 1335708 and bug 1352565, I think. Ok sounds good. In that case a review of part 2 of bug 1335708 would be appreciated, since I'd like to continue working on bug 1352565 (which depends on it) tomorrow. > > > > I don't think we need to cache any other pseudo styles on the ElementData, > > since the post-traveral can find them on the frame. > > Right, the thing that caching allows to do is _efficiently_ knowing if > there's a frame at all. If you keep them cached when we use them, knowing > "is there a first-line frame" around is efficient. Otherwise you need to > guess whether you created a first-line frame at all (which may not be that > hard actually), but you need to walk the frame tree. I think it should be pretty straightforward, since the post-traversal just needs to check for the bit and check the frame type. We may also decide that we actually want to eagerly-cascade ::first-letter/::first-line if the primary style cascades to block-like, but that's likely a separate discussion. > > > Emilio, does this all sound like what you had in mind? If so we can see what > > bz thinks. > > Yes, feedback from boris would be appreciated. Overall because he tends to > find flaws in the different approaches quite fast ;) Ok, NI bz for his thoughts here.

bz and I had a long discussion about this after our call, and came up with a plan that I _think_ handles all the pseudo-element cases, including ::first-letter/::first-line. Would be curious for your feedback emilio: https://docs.google.com/document/d/1Wr8Ec_-i6YA6frRY2uc1cxumG9hcAbc83iZ7eEczu7M/edit?usp=sharing This plan differs from the one described upthread in this bug in that it doesn't require the bitmask. Since everything but ::first-letter/::first-line is NAC-backed, we can handle restyling of those by just traversing the nac subtree. And ::first-letter/::first-line are eager, which means that we'll have all the information anyway. Things will differ for servo, which doesn't have NAC, and thus may still need the pseudo_cache stuff.

Fwiw, I added some comments to the doc mentioned in comment 5.

With bug 1364377, it should pretty much work, modulo removal of the pseudos, which isn't done yet but should be trivial.

This is fixed. If there's any problem with how this works now, should be in different bugs.