Bug 149173

Summary: Web Inspector: command-click in NewTabContentView should open new tab without switching to it
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: 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 Flags
Proposed fix joepeck: review+, bburg: commit-queue?

Description BJ Burg 2015-09-15 11:27:40 PDT
This is how the home screen grids work in FF and Safari.
Comment 1 Radar WebKit Bug Importer 2015-09-15 11:28:04 PDT
<rdar://problem/22705409>
Comment 2 BJ Burg 2015-09-15 13:46:41 PDT
Created attachment 261227 [details]
Proposed fix
Comment 3 Joseph Pecoraro 2015-09-15 14:31:58 PDT
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 4 BJ Burg 2015-09-15 16:42:47 PDT
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
Comment 5 BJ Burg 2015-09-15 16:44:43 PDT
Committed r189828: <http://trac.webkit.org/changeset/189828>