946198 - CSS box-shadow highlighter

Status: NEW

(DevTools :: Inspector, enhancement, P3)

Description

[Patrick Brosset <:pbro>]

Attachment: Screen Shot 2013-12-04 at 2.29.12 PM.png

This is part of the meta bug 942176 which aims at providing information overlays for various CSS properties in order to explain how/why they apply.

This bug is for highlighting box-shadow and especially explaining where/how the various values are applied.

See the attachment for a draft idea of what that could look like, and see the following video that shows it in action: 
http://people.mozilla.org/~pbrosset/box-shadow-highlighter.mov

Comment 1

[Patrick Brosset <:pbro>]

Attachment: index.html

Here is the code I used to create the highlighter seen in the video.
This could easily be integrated with the devtools remote highlighter and shown on box-shadow css property hover in the rule-view and computed-view panels.

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Patrick Brosset [:pbrosset] from comment #1)
> Created attachment 8342336 [details]
> index.html
> 
> Here is the code I used to create the highlighter seen in the video.
> This could easily be integrated with the devtools remote highlighter and
> shown on box-shadow css property hover in the rule-view and computed-view
> panels.

This is very cool!  I believe we will also need to support inset shadows: https://developer.mozilla.org/en-US/docs/Web/CSS/box-shadow.  I wonder if there is a way that we could query the platform to get this information rather than computing it ourselves.  It must all be computed already, and would save us a bunch of code and edge cases.

Comment 3

[Patrick Brosset <:pbro>]

Yes you're right, the code attached doesn't deal with inset shadows yet.
As for computing box shadow, what I did was just parse the computed style string.
To avoid this step, we'd have to get access to the lower level css parser output, which we anyway aim to do to better support authored styles in the future.
In the meantime, I'm not sure if we could get this information from the platform.

Comment 4

[Patrick Brosset <:pbro>]

2 other use cases that should be taken into account:
- multiple shadows (<shadow>, <shadow>, <shadow>)
- when the element is transformed, the shadow is transformed too

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Patrick Brosset [:pbrosset] from comment #3)
> Yes you're right, the code attached doesn't deal with inset shadows yet.
> As for computing box shadow, what I did was just parse the computed style
> string.
> To avoid this step, we'd have to get access to the lower level css parser
> output, which we anyway aim to do to better support authored styles in the
> future.
> In the meantime, I'm not sure if we could get this information from the
> platform.

Cameron, do you know of a way to get the individual components from a CSS box shadow?  In a demo like this: https://bug946198.bugzilla.mozilla.org/attachment.cgi?id=8342336 the width/height/blur/spread/offset are all parsed out of the computed style string.  This will get more complicated in order to support additional features like inset shadows, accounting for edge cases in the spec (if any), etc.  It would be useful if there was a way to gather this information about a box shadow from the platform.

Comment 6

[Cameron McCormack (:heycam)]

Nice demo!  I can see that parsing the string returned by getComputedStyle(element,"").boxShadow isn't trivial -- you can't just split on white space for example -- but it doesn't look too bad.  Are there values that you want that aren't present in this string?  Is there some processing of the values that is not taken into account by the computed value that you want to have applied by this new API?

BTW, you can see what information is stored in the computed style internally for a single shadow item here:

  nsCSSShadowItem
  https://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.h#646

and this is where shadows are drawn:

  nsCSSRendering::PaintBoxShadowOuter
  https://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSRendering.cpp#1018

  nsCSSRendering::PaintBoxShadowInner
  https://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSRendering.cpp#1202

Comment 7

[Cameron McCormack (:heycam)]

Having an API that gives you all the data that's in nsStyleBorder::mBoxShadow as an array of objects with properties corresponding to the things on the nsCSSShadowItem shouldn't be too hard, if that is all you need.

Comment 8

[Brian Grinstead [:bgrins]]

> Nice demo!  I can see that parsing the string returned by
> getComputedStyle(element,"").boxShadow isn't trivial -- you can't just split
> on white space for example -- but it doesn't look too bad. 

> Having an API that gives you all the data that's in
> nsStyleBorder::mBoxShadow as an array of objects with properties
> corresponding to the things on the nsCSSShadowItem shouldn't be too hard, if
> that is all you need.

Thanks for the response.  Yes, I suppose that if we get a consistent serialization of this object from computed style it shouldn't too big a deal to gather these in JavaScript.  Patrick may have an idea whether there would be some additional information that we could use for this or if it would make more sense to get them directly from the mBoxShadow object.

One thing I've noticed after looking through the linked code is that text shadows are using the same type, so we could share whatever code we end up using for this, if we can think of a useful way to visualize text shadows in a tooltip or overlay.

Comment 9

[Patrick Brosset <:pbro>]

Interesting discussion.
I'm starting to agree with Brian's comment 5 in that we may meet more and more complex use cases as we go.

Assuming we find interesting and useful ways to represent various CSS properties, parsing their corresponding computed style strings may get harder.

The thing is, we have no clear view yet on what we want to do apart from this bug and bug 945167 (I also described some other use cases in https://etherpad.mozilla.org/5CDtSdmHu0).

In any case, having a platform API for getting various CSS properties' internal components does sound very handy, simplifying our code, avoiding re-parsing something that's already been parsed. So I'd say, let's go for it!
Cameron: how do you suggest we get started? Should we open a bug for an API to get several CSS properties?

Comment 10

[Patrick Brosset <:pbro>]

Attachment: shadow-highlighter.zip

Here's a new version of the simple demo attached previously in this patch.
This one now takes inset box shadow into account as well as text shadow.

Comment 11

[Brian Grinstead [:bgrins]]

Attachment: Screenshot 2013-12-09 08.05.27.png

The text shadow preview becomes more of a big block when there are multiple lines of text (see screenshot).  I think it would preview well as a tooltip with some sample text and a shadow, maybe Abc like we do for the font inspector.

Comment 12

[Patrick Brosset <:pbro>]

Yes you are right.
On a somewhat related note, I think we will need to work on how best to display the boxes and arrows even for box shadow because it's pretty common to use tiny offset and blur values, which would make it hard to read the labels in the attached demo.

Comment 13

[Cameron McCormack (:heycam)]

(In reply to Patrick Brosset [:pbrosset] from comment #9)
> The thing is, we have no clear view yet on what we want to do apart from
> this bug and bug 945167 (I also described some other use cases in
> https://etherpad.mozilla.org/5CDtSdmHu0).
> 
> In any case, having a platform API for getting various CSS properties'
> internal components does sound very handy, simplifying our code, avoiding
> re-parsing something that's already been parsed. So I'd say, let's go for it!
> Cameron: how do you suggest we get started? Should we open a bug for an API
> to get several CSS properties?

Yes feel free to.  We can begin with exposing exactly what is in the computed style for box-shadow, and then go from there to see if anything else is needed.  (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 14

[Patrick Brosset <:pbro>]

Bug 948428 created

Comment 15

[Patrick Brosset <:pbro>]

Inspector bug triage (filter on CLIMBING SHOES).