Bug 224654 – Web Inspector: Graphics: add support for `steps()`/`spring()` CSS timing functions

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

Uncaught Exception in Web Inspector.

# STEPS TO REPRODUCE
1. load <https://devinrousso.com/projects/?no-pulse&no-background>
2. while the cubes that make up the "DEVIN ROUSSO" text at the top are "exploding", inspect the page


Uncaught Exceptions:
-----------------------
 - TypeError: null is not an object (evaluating 'easing.inPoint') (at AnimationContentView.js:279:32)
    _refreshPreview @ AnimationContentView.js:279:32
    layout @ AnimationContentView.js:91:29
    _layoutSubtree @ View.js:293:20
    _visitViewTreeForLayout @ View.js:392:36
-----------------------

Notes:
Inspected URL:        https://devinrousso.com/projects/?no-pulse&no-background
Loading completed:    true
Frontend User Agent:  Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko)

[Devin Rousso]

I'm pretty sure that the reason this is happening is because `WI.Animation` assumes that the `timingFunction`/`easing` is always a `WI.CubicBezier`.  This site uses CSS `spring()` animations (i.e. `WI.Spring`).

Along those lines, we should also support other timing functions:
 - `steps()`
 - `jump-start`
 - `jump-end`
 - `jump-none`
 - `jump-both`
 - `start`
 - `end`
 - `step-start`
 - `step-end`
(I'd probably just put them all into a single `WI.StepsTimingFunction` or something)

[Devin Rousso]

Created attachment 426351 [details]
Patch

[Devin Rousso]

Created attachment 426352 [details]
[Image] after Patch is applied

[Blaze Burg]

Comment on attachment 426351 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=426351&action=review

r=me

> Source/WebInspectorUI/UserInterface/Models/Geometry.js:600
> +        if (!text?.length)

It would be helpful for future readers if there was a link to the section of the spec where the grammar for this is defined.

> Source/WebInspectorUI/UserInterface/Views/AnimationContentView.js:316
> +                    let stepX = width / easing.count; // always use the defined number of steps to divide the duration

Nit: comment style seems off

> Source/WebInspectorUI/UserInterface/Views/AnimationContentView.js:337
> +                    // FIXME: Is there a simpler way to do this?

I'd remove this.

[Devin Rousso]

Comment on attachment 426351 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=426351&action=review

>> Source/WebInspectorUI/UserInterface/Models/Geometry.js:600
>> +        if (!text?.length)
> 
> It would be helpful for future readers if there was a link to the section of the spec where the grammar for this is defined.

This is used almost 100 times already in `Source/WebInspectorUI` alone.  I don't think we need to add comments whenever new JS grammar is added, especially when that grammar is almost always searchable (e.g. searching for "?." on MDN gives me "Optional Chaining" as the first result).

[Devin Rousso]

Created attachment 426459 [details]
Patch

[Blaze Burg]

(In reply to Devin Rousso from comment #5)
> Comment on attachment 426351 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=426351&action=review
> 
> >> Source/WebInspectorUI/UserInterface/Models/Geometry.js:600
> >> +        if (!text?.length)
> > 
> > It would be helpful for future readers if there was a link to the section of the spec where the grammar for this is defined.
> 
> This is used almost 100 times already in `Source/WebInspectorUI` alone.  I
> don't think we need to add comments whenever new JS grammar is added,
> especially when that grammar is almost always searchable (e.g. searching for
> "?." on MDN gives me "Optional Chaining" as the first result).

Oh, sorry to be vague, I meant the grammar of the property value being parsed.

[EWS]

Committed r276272 (236754@main): <https://commits.webkit.org/236754@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 426459 [details].

[Ryan Haddad]

rdar://76861171