<rdar://75104395>
Created attachment 422612 [details] Patch v1.0
Comment on attachment 422612 [details] 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
Comment on attachment 422612 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=422612&action=review >> 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? Correct. This patch doesn't change that behavior. We do still stop scrolling when you hit stop, and then make one final jump after all the data has come in if we needed to scroll a bit further to account for it (something that began before stopping, but finished after stopping). That logic happens above on (:481-490). >> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:874 >> + timelineView.currentTime = this._currentTime; > > NIT: we normally set `startTime`, then `currentTime`, then `endTime` :P Whoops.
Created attachment 422616 [details] Patch v1.1 - Review notes
Committed r274112: <https://commits.webkit.org/r274112> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422616 [details].