Bug 224654 - Web Inspector: Graphics: add support for `steps()`/`spring()` CSS timing functions
Summary: Web Inspector: Graphics: add support for `steps()`/`spring()` CSS timing func...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL: https://devinrousso.com/projects/?no-...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-15 23:56 PDT by Devin Rousso
Modified: 2021-04-19 18:27 PDT (History)
4 users (show)

See Also:


Attachments
Patch (20.00 KB, patch)
2021-04-17 15:29 PDT, Devin Rousso
bburg: review+
Details | Formatted Diff | Diff
[Image] after Patch is applied (501.18 KB, image/png)
2021-04-17 15:29 PDT, Devin Rousso
no flags Details
Patch (19.92 KB, patch)
2021-04-19 12:19 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2021-04-15 23:56:51 PDT
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)
Comment 1 Devin Rousso 2021-04-16 00:46:33 PDT
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)
Comment 2 Devin Rousso 2021-04-17 15:29:24 PDT
Created attachment 426351 [details]
Patch
Comment 3 Devin Rousso 2021-04-17 15:29:40 PDT
Created attachment 426352 [details]
[Image] after Patch is applied
Comment 4 BJ Burg 2021-04-19 12:13:39 PDT
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.
Comment 5 Devin Rousso 2021-04-19 12:17:48 PDT
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).
Comment 6 Devin Rousso 2021-04-19 12:19:53 PDT
Created attachment 426459 [details]
Patch
Comment 7 BJ Burg 2021-04-19 12:27:22 PDT
(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.
Comment 8 EWS 2021-04-19 13:46:06 PDT
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].
Comment 9 Ryan Haddad 2021-04-19 18:27:41 PDT
rdar://76861171