[webkit-reviews] review granted: [Bug 222921] Web Inspector: REGRESSION (r267038): Inspector fails to show any details about animations/transitions in the timeline : [Attachment 422612] Patch v1.0

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Patrick Angle <pangle at apple.com>'s
request for review:
Bug 222921: Web Inspector: REGRESSION (r267038): Inspector fails to show any
details about animations/transitions in the timeline
https://bugs.webkit.org/show_bug.cgi?id=222921

Attachment 422612: Patch v1.0

https://bugs.webkit.org/attachment.cgi?id=422612&action=review




--- Comment #2 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 422612
  --> https://bugs.webkit.org/attachment.cgi?id=422612
Patch v1.0

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

r=me

> Source/WebInspectorUI/ChangeLog:13
> +	   The check for not being in the inactive state was particularlly
problematic as it is quite common that a

"particularlly"

>
Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:-875
> -	       // Only update end time while not stopping, otherwise the
interface contues scrolling.

So just to make sure, the "otherwise the interface contues scrolling" part is
no longer the case, right?  When stopping we don't continue scrolling, yes?

>
Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:-879
> -		   // Only update current time while active/starting or else
the interface continues scrolling.

ditto (:-875)

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:874
> +	   timelineView.endTime = endTime;
> +	   timelineView.currentTime = this._currentTime;

NIT: we normally set `startTime`, then `currentTime`, then `endTime` :P