1268748 - Implement {Resolve,Probe}PseudoElementStyle

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Bobby Holley (:bholley)]

The servo side doesn't yet have the machinery to resolve non-eager selector-based pseudos, so I've hacked it up in my local tree to do something simple. Emilio said he'd look at making the required servo changes.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Implement {Resolve,Probe}PseudoElementStyle. v1

Comment 2

[Cameron McCormack (:heycam)]

Comment on attachment 8746897 [details] [diff] [review]
Implement {Resolve,Probe}PseudoElementStyle. v1

Review of attachment 8746897 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/ServoBindings.cpp
@@ +312,5 @@
>    MOZ_CRASH("stylo: shouldn't be calling Servo_GetComputedValuesForAnonymousBox in a "
>              "non-MOZ_STYLO build");
>  }
>  
> +ServoComputedValues* Servo_GetComputedValuesForPseudoElement(ServoComputedValues* parent_style,

Nit: newline before function name.  (I missed mentioning this in some of the other functions in this file.)

::: layout/style/ServoStyleSet.cpp
@@ +148,5 @@
>                                           nsStyleContext* aParentContext,
>                                           Element* aPseudoElement)
>  {
> +  MOZ_ASSERT(!aPseudoElement,
> +             "We don't support CSS_PSEUDO_ELEMENT_SUPPORTS_USER_ACTION_STATE yet");

"stylo: ..." for the message, just to be consistent with other assertions for missing stylo functionality we know we'll need to fix.

@@ +154,5 @@
> +  MOZ_ASSERT(aType < CSSPseudoElementType::Count);
> +  nsIAtom* pseudoTag = nsCSSPseudoElements::GetPseudoAtom(aType);
> +
> +  RefPtr<ServoComputedValues> computedValues =
> +    Servo_GetComputedValuesForPseudoElement(aParentContext->StyleSource().AsServoComputedValues(),

These long names (which I like) don't play well with 80 column limits (which I don't like, but try to keep to).  How about wrapping this as:

  RefPtr<SevoComputedValues> computedValues =
    Servo_GetComputedValuesForPseudoElement(
      aParentContext->StyleSource().AsServoComputedValues(),
      ...);

@@ +325,5 @@
> +  MOZ_ASSERT(aType < CSSPseudoElementType::Count);
> +  nsIAtom* pseudoTag = nsCSSPseudoElements::GetPseudoAtom(aType);
> +
> +  RefPtr<ServoComputedValues> computedValues =
> +    Servo_GetComputedValuesForPseudoElement(aParentContext->StyleSource().AsServoComputedValues(),

Here too.

@@ +328,5 @@
> +  RefPtr<ServoComputedValues> computedValues =
> +    Servo_GetComputedValuesForPseudoElement(aParentContext->StyleSource().AsServoComputedValues(),
> +                                            aParentElement, pseudoTag, mRawSet.get(),
> +                                            /* is_probe = */ true);
> +  if (!computedValues) {

Though I think, as I mentioned, it is probably only an optimization, since it is easy please do duplicate the logic from nsStyleSet::ProbePseudoElementStyle that returns null if the display or content properties are none when we're probing for ::before/::after style.

@@ +343,5 @@
>                                         TreeMatchContext& aTreeMatchContext,
>                                         Element* aPseudoElement)
>  {
> +  MOZ_ASSERT(!aPseudoElement,
> +             "We don't support CSS_PSEUDO_ELEMENT_SUPPORTS_USER_ACTION_STATE yet");

"stylo: ..." here too (or just rely on the assertion from the call to the other ProbePseudoElementStyle).

Comment 3

[Bobby Holley (:bholley)]

Attachment: Implement {Resolve,Probe}PseudoElementStyle. v2

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/774c838c19e2

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/774c838c19e2