195594 2019-03-11 18:20:41 -0700 Web Inspector: Timelines - Improve handling of past recordings (readonly) 2019-03-12 00:12:17 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck commit-queue hi inspector-bugzilla-changes joepeck webkit-bug-importer oldest_to_newest 1515167 0 joepeck 2019-03-11 18:20:41 -0700 Timelines - Improve handling of past recordings (readonly) Steps to Reproduce: 1. Inspect a page 2. Show Timelines tab 3. Reload + stop recording => Timeline Recording 1 4. Reload + stop recording => Timeline Recording 2 5. Switch to Timeline Recording 1 => Usability issues Issue: • Ability to start a recording continues this readonly recording... should probably not • Ability to clear recording with Cmd+K keyboard shortcut (button was disabled) 1515168 1 364330 joepeck 2019-03-11 18:22:40 -0700 Created attachment 364330 [PATCH] Proposed Fix 1515189 2 364330 hi 2019-03-11 19:09:47 -0700 Comment on attachment 364330 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=364330&action=review rs=me > Source/WebInspectorUI/UserInterface/Views/TimelineTabContentView.js:487 > + if (WI.timelineManager.activeRecording.readonly) > + shouldCreateNewRecording = true; Should we also change the tooltip of the record button to remove the non-`shouldCreateNewRecording` text? e.g. `"Start recording (%s)\nCreate new recording (%s)"` becomes `"Create new recording (%s)"` when the active recording is readonly (and vice versa). 1515332 3 joepeck 2019-03-11 23:43:45 -0700 (In reply to Devin Rousso from comment #2) > Comment on attachment 364330 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=364330&action=review > > rs=me > > > Source/WebInspectorUI/UserInterface/Views/TimelineTabContentView.js:487 > > + if (WI.timelineManager.activeRecording.readonly) > > + shouldCreateNewRecording = true; > > Should we also change the tooltip of the record button to remove the > non-`shouldCreateNewRecording` text? e.g. `"Start recording (%s)\nCreate > new recording (%s)"` becomes `"Create new recording (%s)"` when the active > recording is readonly (and vice versa). Naw, I don't think this is common enough to warrant it. I think the current tooltip is still reasonably accurate even in the readonly case, it will start a recording! 1515345 4 364330 commit-queue 2019-03-12 00:11:03 -0700 Comment on attachment 364330 [PATCH] Proposed Fix Clearing flags on attachment: 364330 Committed r242785: <https://trac.webkit.org/changeset/242785> 1515346 5 commit-queue 2019-03-12 00:11:05 -0700 All reviewed patches have been landed. Closing bug. 1515347 6 webkit-bug-importer 2019-03-12 00:12:17 -0700 <rdar://problem/48799997> 364330 2019-03-11 18:22:40 -0700 2019-03-12 00:11:03 -0700 [PATCH] Proposed Fix readonly-1.patch text/plain 2914 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IDFhNThjMzg5ZjUxLi5iZTQ3MDFmODE4MSAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjEgQEAKKzIwMTktMDMtMTEgIEpvc2Vw aCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKworICAgICAgICBXZWIgSW5zcGVjdG9y OiBUaW1lbGluZXMgLSBJbXByb3ZlIGhhbmRsaW5nIG9mIHBhc3QgcmVjb3JkaW5ncyAocmVhZG9u bHkpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTU1 OTQKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVz ZXJJbnRlcmZhY2UvVmlld3MvVGltZWxpbmVSZWNvcmRpbmdDb250ZW50Vmlldy5qczoKKyAgICAg ICAgKFdJLlRpbWVsaW5lUmVjb3JkaW5nQ29udGVudFZpZXcucHJvdG90eXBlLl9jbGVhclRpbWVs aW5lKToKKyAgICAgICAgRG9uJ3QgYWxsb3cgY2xlYXJpbmcgb2YgYSByZWFkb25seSByZWNvcmRp bmcuCisKKyAgICAgICAgKiBVc2VySW50ZXJmYWNlL1ZpZXdzL1RpbWVsaW5lVGFiQ29udGVudFZp ZXcuanM6CisgICAgICAgIChXSS5UaW1lbGluZVRhYkNvbnRlbnRWaWV3LnByb3RvdHlwZS5fdG9n Z2xlUmVjb3JkaW5nT25TcGFjZWJhcik6CisgICAgICAgIERvbid0IGRvIGFueXRoaW5nIHdoZW4g dmlld2luZyBhIHJlYWRvbmx5IHJlY29yZGluZy4KKworICAgICAgICAoV0kuVGltZWxpbmVUYWJD b250ZW50Vmlldy5wcm90b3R5cGUuX3JlY29yZEJ1dHRvbkNsaWNrZWQpOgorICAgICAgICBTdGFy dCBhIG5ldyByZWNvcmRpbmcgaWYgdmlld2luZyBhIHJlYWRvbmx5IHJlY29yZGluZy4KKwogMjAx OS0wMy0xMSAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUuY29tPgogCiAgICAgICAg IFdlYiBJbnNwZWN0b3I6IEVkaXRpbmcgVGltZWxpbmVzIHNob3dzIHR3byBDUFUgVGltZWxpbmVz CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9U aW1lbGluZVJlY29yZGluZ0NvbnRlbnRWaWV3LmpzIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1Vz ZXJJbnRlcmZhY2UvVmlld3MvVGltZWxpbmVSZWNvcmRpbmdDb250ZW50Vmlldy5qcwppbmRleCA5 YWRiNzk3Y2ExMi4uMjMyMmI3MjE0YTMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JV SS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1RpbWVsaW5lUmVjb3JkaW5nQ29udGVudFZpZXcuanMKKysr IGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvVGltZWxpbmVSZWNv cmRpbmdDb250ZW50Vmlldy5qcwpAQCAtNTY0LDYgKzU2NCw5IEBAIFdJLlRpbWVsaW5lUmVjb3Jk aW5nQ29udGVudFZpZXcgPSBjbGFzcyBUaW1lbGluZVJlY29yZGluZ0NvbnRlbnRWaWV3IGV4dGVu ZHMgV0kuCiAKICAgICBfY2xlYXJUaW1lbGluZShldmVudCkKICAgICB7CisgICAgICAgIGlmICh0 aGlzLl9yZWNvcmRpbmcucmVhZG9ubHkpCisgICAgICAgICAgICByZXR1cm47CisKICAgICAgICAg aWYgKFdJLnRpbWVsaW5lTWFuYWdlci5hY3RpdmVSZWNvcmRpbmcgPT09IHRoaXMuX3JlY29yZGlu ZyAmJiBXSS50aW1lbGluZU1hbmFnZXIuaXNDYXB0dXJpbmcoKSkKICAgICAgICAgICAgIFdJLnRp bWVsaW5lTWFuYWdlci5zdG9wQ2FwdHVyaW5nKCk7CiAKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJ bnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1RpbWVsaW5lVGFiQ29udGVudFZpZXcuanMg Yi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9UaW1lbGluZVRhYkNv bnRlbnRWaWV3LmpzCmluZGV4IGRiMTM5Yjg3NTdkLi40ZDA4NWY3ZDk5NCAxMDA2NDQKLS0tIGEv U291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvVGltZWxpbmVUYWJDb250 ZW50Vmlldy5qcworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3 cy9UaW1lbGluZVRhYkNvbnRlbnRWaWV3LmpzCkBAIC00NDUsNiArNDQ1LDkgQEAgV0kuVGltZWxp bmVUYWJDb250ZW50VmlldyA9IGNsYXNzIFRpbWVsaW5lVGFiQ29udGVudFZpZXcgZXh0ZW5kcyBX SS5Db250ZW50QnJvd3MKIAogICAgIF90b2dnbGVSZWNvcmRpbmdPblNwYWNlYmFyKGV2ZW50KQog ICAgIHsKKyAgICAgICAgaWYgKFdJLnRpbWVsaW5lTWFuYWdlci5hY3RpdmVSZWNvcmRpbmcucmVh ZG9ubHkpCisgICAgICAgICAgICByZXR1cm47CisKICAgICAgICAgaWYgKFdJLmlzRXZlbnRUYXJn ZXRBbkVkaXRhYmxlRmllbGQoZXZlbnQpKQogICAgICAgICAgICAgcmV0dXJuOwogCkBAIC00ODAs NiArNDgzLDkgQEAgV0kuVGltZWxpbmVUYWJDb250ZW50VmlldyA9IGNsYXNzIFRpbWVsaW5lVGFi Q29udGVudFZpZXcgZXh0ZW5kcyBXSS5Db250ZW50QnJvd3MKICAgICBfcmVjb3JkQnV0dG9uQ2xp Y2tlZChldmVudCkKICAgICB7CiAgICAgICAgIGxldCBzaG91bGRDcmVhdGVOZXdSZWNvcmRpbmcg PSB3aW5kb3cuZXZlbnQgPyB3aW5kb3cuZXZlbnQuc2hpZnRLZXkgOiBmYWxzZTsKKyAgICAgICAg aWYgKFdJLnRpbWVsaW5lTWFuYWdlci5hY3RpdmVSZWNvcmRpbmcucmVhZG9ubHkpCisgICAgICAg ICAgICBzaG91bGRDcmVhdGVOZXdSZWNvcmRpbmcgPSB0cnVlOworCiAgICAgICAgIHRoaXMuX3Jl Y29yZEJ1dHRvbi50b2dnbGVkID0gIVdJLnRpbWVsaW5lTWFuYWdlci5pc0NhcHR1cmluZygpOwog ICAgICAgICB0aGlzLl90b2dnbGVSZWNvcmRpbmcoc2hvdWxkQ3JlYXRlTmV3UmVjb3JkaW5nKTsK ICAgICB9Cg==