| Summary: | Web Inspector: command-click in NewTabContentView should open new tab without switching to it | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||
| Component: | Web Inspector | Assignee: | BJ Burg <bburg> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | bburg, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
BJ Burg
2015-09-15 11:27:40 PDT
Created attachment 261227 [details]
Proposed fix
Comment on attachment 261227 [details] Proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=261227&action=review r=me > Source/WebInspectorUI/UserInterface/Base/Main.js:510 > +WebInspector.createNewTabWithType = function(tabType, options={}) In the one other place I think we have this right now, we have spaces. I still favor no spaces. Lets decide on IRC. > Source/WebInspectorUI/UserInterface/Base/Main.js:520 > + let insertionIndex = referencedView ? this.tabBar.tabBarItems.indexOf(referencedView.tabBarItem) : null; Nit: I think `undefined` may be better than `null`. Null makes me think an object should go here, but it should be a number (or a default value), which null is not typically associated with. > Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js:114 > + let tabItemElements = Array.from(this.element.querySelectorAll("." + WebInspector.NewTabContentView.TabItemStyleClassName)); Nit: This could be a private function, since it is done in multiple places and is easy to get wrong. Comment on attachment 261227 [details] Proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=261227&action=review >> Source/WebInspectorUI/UserInterface/Base/Main.js:520 >> + let insertionIndex = referencedView ? this.tabBar.tabBarItems.indexOf(referencedView.tabBarItem) : null; > > Nit: I think `undefined` may be better than `null`. Null makes me think an object should go here, but it should be a number (or a default value), which null is not typically associated with. OK Committed r189828: <http://trac.webkit.org/changeset/189828> |