201539 2019-09-06 01:50:17 -0700 Web Inspector: Better position for Sources tab when enabling the experimental setting 2019-09-06 14:21:17 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck hi inspector-bugzilla-changes joepeck webkit-bug-importer oldest_to_newest 1568444 0 joepeck 2019-09-06 01:50:17 -0700 Better position for Sources tab when enabling the experimental setting It is horrible that the new Sources tab ends up all the way at the end. Let's try to place it where the Debugger tab was. 1568446 1 378171 joepeck 2019-09-06 01:51:11 -0700 Created attachment 378171 [PATCH] Proposed Fix 1568597 2 378171 hi 2019-09-06 11:10:09 -0700 Comment on attachment 378171 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=378171&action=review r=me > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:348 > let newTabs = WI._openTabsSetting.value.slice(); NIT: now that <https://webkit.org/b/17240> added `WI.Setting.prototype.save`, we can avoid this `slice` and instead just forcibly call `WI._openTabsSetting.save()` at the end of this function, right before `InspectorFrontendHost.reopen()`. > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:352 > + let index = newTabs.indexOf(WI.DebuggerTabContentView.Type); Rather than look for `WI.DebuggerTabContentVIew` specifically, perhaps we could expose `productionTabClasses` (in `WI.contentLoaded` in Main.js) and use that for ordering (which would also benefit the Layers Tab), since that already has a defined order of where the default tabs should be. This way, if the user doesn't have the Debugger Tab open, but one of the other "nearby" tabs is open, we'd still add the Sources Tab in the "generally" right place :) > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:354 > + newTabs.splice(index, 0, WI.SourcesTabContentView.Type); NIT: `Array.prototype.insertAtIndex`. 1568690 3 378171 joepeck 2019-09-06 14:09:18 -0700 Comment on attachment 378171 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=378171&action=review >> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:352 >> + let index = newTabs.indexOf(WI.DebuggerTabContentView.Type); > > Rather than look for `WI.DebuggerTabContentVIew` specifically, perhaps we could expose `productionTabClasses` (in `WI.contentLoaded` in Main.js) and use that for ordering (which would also benefit the Layers Tab), since that already has a defined order of where the default tabs should be. > > This way, if the user doesn't have the Debugger Tab open, but one of the other "nearby" tabs is open, we'd still add the Sources Tab in the "generally" right place :) I'm not sure how we would decide to reconcile all cases. So I've simplified to this: • If the Debugger tab exists, no matter where the user ordered it, use that spot • If the Debugger tab does not exist, then the user has already shows ability to customized tabs. In any case, this is a short term solution until we enable it by default! >> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:354 >> + newTabs.splice(index, 0, WI.SourcesTabContentView.Type); > > NIT: `Array.prototype.insertAtIndex`. Nice! 1568694 4 joepeck 2019-09-06 14:20:13 -0700 https://trac.webkit.org/changeset/249592/webkit 1568697 5 webkit-bug-importer 2019-09-06 14:21:17 -0700 <rdar://problem/55127412> 378171 2019-09-06 01:51:11 -0700 2019-09-06 14:09:18 -0700 [PATCH] Proposed Fix sources-tab-index.patch text/plain 2425 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IDllYzY5ZGVkMjU3Li4zYjI1Zjc0ODU2MCAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTktMDktMDYgIEpvc2Vw aCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKworICAgICAgICBXZWIgSW5zcGVjdG9y OiBCZXR0ZXIgcG9zaXRpb24gZm9yIFNvdXJjZXMgdGFiIHdoZW4gZW5hYmxpbmcgdGhlIGV4cGVy aW1lbnRhbCBzZXR0aW5nCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0yMDE1MzkKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICAqIFVzZXJJbnRlcmZhY2UvVmlld3MvU2V0dGluZ3NUYWJDb250ZW50Vmlldy5qczoK KyAgICAgICAgUGxhY2UgdGhlIFNvdXJjZXMgdGFiIHdoZXJlIHRoZSBEZWJ1Z2dlciB0YWIgd2Fz LgorCiAyMDE5LTA5LTA2ICBKb3NlcGggUGVjb3Jhcm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CiAK ICAgICAgICAgV2ViIEluc3BlY3RvcjogTmV0d29yazogRXJyb3IgcmVzcG9uc2UgaGVhZGVycyBz dW1tYXJ5IGhhcyB3cm9uZyBjb2xvciBib3JkZXIKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNw ZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NldHRpbmdzVGFiQ29udGVudFZpZXcuanMgYi9T b3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9TZXR0aW5nc1RhYkNvbnRl bnRWaWV3LmpzCmluZGV4IDNmYjgzOTlhMDQ4Li4zNTI2NDM4YWViNyAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvU2V0dGluZ3NUYWJDb250ZW50 Vmlldy5qcworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9T ZXR0aW5nc1RhYkNvbnRlbnRWaWV3LmpzCkBAIC0zNDYsOCArMzQ2LDE2IEBAIFdJLlNldHRpbmdz VGFiQ29udGVudFZpZXcgPSBjbGFzcyBTZXR0aW5nc1RhYkNvbnRlbnRWaWV3IGV4dGVuZHMgV0ku VGFiQ29udGVudFZpCiAgICAgICAgIHJlbG9hZEluc3BlY3RvckJ1dHRvbi5hZGRFdmVudExpc3Rl bmVyKCJjbGljayIsIChldmVudCkgPT4gewogICAgICAgICAgICAgLy8gRm9yY2UgYSBjb3B5IHNv IHRoYXQgV0kuU2V0dGluZyBzZWVzIGl0IGFzIGEgbmV3IHZhbHVlLgogICAgICAgICAgICAgbGV0 IG5ld1RhYnMgPSBXSS5fb3BlblRhYnNTZXR0aW5nLnZhbHVlLnNsaWNlKCk7Ci0gICAgICAgICAg ICBpZiAoIWluaXRpYWxWYWx1ZXMuZ2V0KFdJLnNldHRpbmdzLmV4cGVyaW1lbnRhbEVuYWJsZVNv dXJjZXNUYWIpICYmIFdJLnNldHRpbmdzLmV4cGVyaW1lbnRhbEVuYWJsZVNvdXJjZXNUYWIudmFs dWUpCi0gICAgICAgICAgICAgICAgbmV3VGFicy5wdXNoKFdJLlNvdXJjZXNUYWJDb250ZW50Vmll dy5UeXBlKTsKKyAgICAgICAgICAgIGlmICghaW5pdGlhbFZhbHVlcy5nZXQoV0kuc2V0dGluZ3Mu ZXhwZXJpbWVudGFsRW5hYmxlU291cmNlc1RhYikgJiYgV0kuc2V0dGluZ3MuZXhwZXJpbWVudGFs RW5hYmxlU291cmNlc1RhYi52YWx1ZSkgeworICAgICAgICAgICAgICAgIGxldCBleGlzdGluZ0lu ZGV4ID0gbmV3VGFicy5pbmRleE9mKFdJLlNvdXJjZXNUYWJDb250ZW50Vmlldy5UeXBlKTsKKyAg ICAgICAgICAgICAgICBpZiAoZXhpc3RpbmdJbmRleCA9PT0gLTEpIHsKKyAgICAgICAgICAgICAg ICAgICAgbGV0IGluZGV4ID0gbmV3VGFicy5pbmRleE9mKFdJLkRlYnVnZ2VyVGFiQ29udGVudFZp ZXcuVHlwZSk7CisgICAgICAgICAgICAgICAgICAgIGlmIChpbmRleCAhPT0gLTEpCisgICAgICAg ICAgICAgICAgICAgICAgICBuZXdUYWJzLnNwbGljZShpbmRleCwgMCwgV0kuU291cmNlc1RhYkNv bnRlbnRWaWV3LlR5cGUpOworICAgICAgICAgICAgICAgICAgICBlbHNlCisgICAgICAgICAgICAg ICAgICAgICAgICBuZXdUYWJzLnB1c2goV0kuU291cmNlc1RhYkNvbnRlbnRWaWV3LlR5cGUpOyAg ICAgICAgICAgICAgICAgICAgCisgICAgICAgICAgICAgICAgfQorICAgICAgICAgICAgfQogICAg ICAgICAgICAgaWYgKCFpbml0aWFsVmFsdWVzLmdldChXSS5zZXR0aW5ncy5leHBlcmltZW50YWxF bmFibGVMYXllcnNUYWIpICYmIHdpbmRvdy5MYXllclRyZWVBZ2VudCAmJiBXSS5zZXR0aW5ncy5l eHBlcmltZW50YWxFbmFibGVMYXllcnNUYWIudmFsdWUpCiAgICAgICAgICAgICAgICAgbmV3VGFi cy5wdXNoKFdJLkxheWVyc1RhYkNvbnRlbnRWaWV3LlR5cGUpOwogICAgICAgICAgICAgV0kuX29w ZW5UYWJzU2V0dGluZy52YWx1ZSA9IG5ld1RhYnM7Cg==