1372464 - stylo: Inspector doesn't show style for pseudo-elements

Status: RESOLVED FIXED

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

Description

[Xidorn Quan [:xidorn] UTC+10]

This is because in the Stylo branch of inDOMUtils::GetCSSStyleRules [1] we don't handle pseudo element at all.

We need to pass "pseudoElt" variable to Servo side and get the corresponding rules from there.

[1] https://dxr.mozilla.org/mozilla-central/rev/2a63a6c35033b5cbc6c98cabc7657c7290284691/layout/inspector/inDOMUtils.cpp#274

Comment 1

[Bobby Holley (:bholley)]

Relatively user-visible functionality loss, marking p1.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

I can work on this after bug 1359217. Shouldn't be too hard.

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

This is probably not as simple as I thought.

This is simple for eager pseudo-elements, because their style data is stored together with that of element.

It is also not too hard for pseudo-elements which have corresponding native anonymous content element. We can just query the style for them.

However, there are still several pseudo-elements which are neither eager nor with NAC element. That includes things like ::-moz-list-{number,bullet}, ::backdrop, and ::-moz-selection.

heycam told me that we would always have rule node if we ever resolve a style context for them. But there doesn't seem to be any easy way to get them.

We would need this because DevTools supports listing all pseudo-elements defined for an element when some setting is enabled. See [1].


[1] https://dxr.mozilla.org/mozilla-central/rev/95543bdc59bd038a3d5d084b85a4fec493c349ee/devtools/server/actors/styles.js#540-548

Comment 4

[Bobby Holley (:bholley)]

In bug 1368291 and bug 1368290, we plan to hang a cache of all the text node, anonymous box, and lazy pseudo style contexts off the primary style context. Would that be enough to solve this problem? These style contexts can still be shared across multiple elements (via the style sharing cache), but presumably in those cases we'd already be matching all the same rules.

Comment 5

[Xidorn Quan [:xidorn] UTC+10]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #4)
> In bug 1368291 and bug 1368290, we plan to hang a cache of all the text
> node, anonymous box, and lazy pseudo style contexts off the primary style
> context. Would that be enough to solve this problem? These style contexts
> can still be shared across multiple elements (via the style sharing cache),
> but presumably in those cases we'd already be matching all the same rules.

That would probably be helpful.

It seems to me currently we do compute every pseudo-element for any element from devtools in Gecko, so it is probably fine to just do the same in Stylo.

inDOMUtils::GetCSSStyleRules currently calls into nsComputedDOMStyle to get style context, and the latter recomputes the style for whatever it is queried. It would be wasteful if we end up recomputing that again in Servo side.

That says, if we can get all information from style context, we would be able to share most of the code. Otherwise, we should probably avoid calling into nsComputedDOMStyle and just ask Servo to compute everything directly.

Comment 6

[Xidorn Quan [:xidorn] UTC+10]

Looking at the code, it seems to me this can be fixed more trivially. We just need to get the rule node from the ComputedValues of the style context. Not sure why we use element at the beginning.

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1372464 - Use ComputedValues rather than element to get style rule list.

Review commit: https://reviewboard.mozilla.org/r/155544/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/155544/

Comment 8

[Cameron McCormack (:heycam)]

Comment on attachment 8884682 [details]
Bug 1372464 - Use ComputedValues rather than element to get style rule list.

https://reviewboard.mozilla.org/r/155544/#review160544

Comment 9

[Xidorn Quan [:xidorn] UTC+10]

Servo side: servo/servo#17646

Comment 10

[Xidorn Quan [:xidorn] UTC+10]

Comment on attachment 8884682 [details]
Bug 1372464 - Use ComputedValues rather than element to get style rule list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155544/diff/1-2/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/265b1327fc07
Use ComputedValues rather than element to get style rule list. r=heycam

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/265b1327fc07