948428 - API to expose box-shadow computed components

Status: NEW

(Core :: CSS Parsing and Computation, enhancement)

Description

[Patrick Brosset <:pbro>]

This bug is created because of bug 946198.
The goal is to expose an API, easily accessible from the FirefoxDevTools, to access individual css box-shadow components for a given node.

Indeed, in bug 946198, we aim at providing useful information in the devtools for complex CSS properties like box-shadow (but not limited to it).
The simple demo attached in that bug basically reads the computed style from the node and parses it to get the offset, blur, spread, color, inset values.

Since these values are already known (see https://bugzilla.mozilla.org/show_bug.cgi?id=946198#c6), it makes sense to expose them via a js API to avoid re-parsing from the string.

As mentioned by Cameron in that bug: 
> Ideally, CSSOM would grow features that would allow us to get easier access to
> the components of the box-shadow computed value, and we wouldn't need this API

Comment 1

[Patrick Brosset <:pbro>]

Feel free to update the product and component if I didn't choose the right one.

Comment 2

[Cameron McCormack (:heycam)]

Boris, to expose shadow property information in a more useful form for devtools folks, do you think it would be OK to add some [ChromeOnly] extensions to CSSStyleDeclaration?  I was thinking of something like this:

  dictionary CSSShadowItem {
    float xOffset;
    float yOffset;
    float radius;
    float spread;
    DOMString color;
    boolean inset;
  };

  partial interface CSSStyleDeclaration {
    [ChromeOnly,Throws]
    sequence<CSSShadowItem> getPropertyValueAsShadowArray(DOMString property);
  };

where you could pass "box-shadow" or "text-shadow" as the property.

Or, should I keep this kind of thing on one of the layout/inspector/in* objects instead?

Comment 3

[Boris Zbarsky [:bzbarsky]]

So the main caveat about adding things to CSSStyleDeclaration is that we have two distinct implementations.  Do we want this sort of thing for both specified and computed styles?

Past that, given that we have the option for [ChromeOnly] stuff now, I think it's fine to do that.

Comment 4

[Cameron McCormack (:heycam)]

I think we might want two separate implementations anyway; for the specified style, we need to extract the right components out of an nsCSSValueList, possibly not putting each member on the returned dictionaries, while for the computed style we need to grab the information off the style structs.

Comment 5

[Cameron McCormack (:heycam)]

I'm assuming it's useful to get information in this form for both specified and compute styles: Patrick, do you need both?

If we only wanted to support say computed styles, I guess I might do something like

  [NoInterfaceObject]
  interface ComputedStyleStuff {
    ...
  };

  CSS2Properties implements ComputedStyleStuff;

?

Comment 6

[Boris Zbarsky [:bzbarsky]]

> I think we might want two separate implementations anyway

My point is that if we only need this information on specified styles or only on computed styles then it makes less sense to put it on CSSStyleDeclaration.  If we'll want it for both, might as well put it on that interface.

> CSS2Properties implements ComputedStyleStuff;

CSS2Properties is implemented by both specified and computed styles.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Providing this API seems like it'll take quite a bit of code; it's not clear to me what the value is over the existing string representation.  The thread in bug 946198 seems to suggest that you're thinking we want to do similar things for other properties.  If we do, it seems like we ought to have a simpler way of doing it.

Comment 8

[Cameron McCormack (:heycam)]

(In reply to Boris Zbarsky [:bz] from comment #6)
> > I think we might want two separate implementations anyway
> 
> My point is that if we only need this information on specified styles or
> only on computed styles then it makes less sense to put it on
> CSSStyleDeclaration.  If we'll want it for both, might as well put it on
> that interface.

OK.

> > CSS2Properties implements ComputedStyleStuff;
> 
> CSS2Properties is implemented by both specified and computed styles.

Oh right.  I guess there's no handy interface to hang it off then, if we only want it for specified or computed style declarations.

(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #7)
> Providing this API seems like it'll take quite a bit of code; it's not clear
> to me what the value is over the existing string representation.  The thread
> in bug 946198 seems to suggest that you're thinking we want to do similar
> things for other properties.  If we do, it seems like we ought to have a
> simpler way of doing it.

Dunno about quite a bit of code.

I don't think we would need to provide an API for all properties; just ones that are (a) difficult to parse out of a string, and (b) devtools needs to do something with.  I feel like the more general or simpler approach would end being to wait for CSSOM to provide this, or perhaps implement the getPropertyCSSValue stuff and make some extensions there ourselves.

Comment 9

[Patrick Brosset <:pbro>]

(In reply to Cameron McCormack (:heycam) from comment #5)
> I'm assuming it's useful to get information in this form for both specified
> and compute styles: Patrick, do you need both?
As of now, we only need it for computed style, since we're looking at using this information to highlight things in the content document, so box-shadow that actually applied to the element.
I can't be positive however that we're not going to need it for specified styles too.

Comment 10

[Patrick Brosset <:pbro>]

(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #7)
> Providing this API seems like it'll take quite a bit of code; it's not clear
> to me what the value is over the existing string representation.  
The value for the devtools code is to avoid parsing the sometimes not so trivial computed style string.

(In reply to Cameron McCormack (:heycam) from comment #8)
> I don't think we would need to provide an API for all properties; just ones
> that are (a) difficult to parse out of a string, and (b) devtools needs to
> do something with.  I feel like the more general or simpler approach would
> end being to wait for CSSOM to provide this, or perhaps implement the
> getPropertyCSSValue stuff and make some extensions there ourselves.
Agreed, (a) and (b) make sense.

Comment 11

[Boris Zbarsky [:bzbarsky]]

> I guess there's no handy interface to hang it off then

Right.  If we only want it for one of those, inIDOMUtils is perhaps the way to go.  Or I guess just making one of the impls a no-op.