Bug 183317 – Web Inspector: Sources: Open all resources in Sources tab instead of Resources/Debugger

• Component: Web Inspector
• Status: RESOLVED FIXED

[Nikita Vasilyev]

When Sources tab is enabled, all resources that currently open in Resources or Debugger should open in Sources tab instead.

[Radar WebKit Bug Importer]

<rdar://problem/38108455>

[Nikita Vasilyev]

Some ways to open a resource:
- Click on a source link
  - from a DOM outline
  - from the styles editor
  - from a console message
- Quick open (Command-Shift-O)
- Search
- Timelines

[Nikita Vasilyev]

Created attachment 335474 [details]
Patch

[Nikita Vasilyev]

Created attachment 335476 [details]
Patch

[Matt Baker]

Comment on attachment 335476 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335476&action=review

> Source/WebInspectorUI/UserInterface/Views/ContextMenuUtilities.js:107
> +                    showResourceWithOptions({tabIdentifier: WI.SourcesTabContentView.Identifier});

I like this idea, but what do you think about renaming the parameter `tabType`? WI.SourcesTabContentView.Type already exists, and as far as I know a TabContentView's type and identifier are always the same.

> Source/WebInspectorUI/UserInterface/Views/TabBrowser.js:130
> +                    return tabContentView;

Since this code will fall back on recentTabContentViews if a matching tab isn't found, should we rename the option `preferredTabType`?

[Nikita Vasilyev]

Created attachment 335485 [details]
Patch

[Matt Baker]

Comment on attachment 335485 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335485&action=review

> Source/WebInspectorUI/UserInterface/Views/TabBrowser.js:125
> +            for (let tabContentView of this._recentTabContentViews) {

Sorry for not mentioning this sooner (our changes collided as I added it):

Nit: Array.prototype.find with an inline arrow is more concise and less error-prone than the equivalent for-loop:

let tabContentView = this._recentTabContentViews.find((tabContentView) => tabContentView.type === options.preferredTabType);
if (tabContentView)
    return tabContentView;

[Devin Rousso]

Comment on attachment 335476 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=335476&action=review

> Source/WebInspectorUI/UserInterface/Base/Main.js:976
> +WI.showSourcesTab = function(options)

This should be `options = {}` if we ever want to call `WI.showSourcesTab()` without arguments.

>> Source/WebInspectorUI/UserInterface/Views/ContextMenuUtilities.js:107
>> +                    showResourceWithOptions({tabIdentifier: WI.SourcesTabContentView.Identifier});
> 
> I like this idea, but what do you think about renaming the parameter `tabType`? WI.SourcesTabContentView.Type already exists, and as far as I know a TabContentView's type and identifier are always the same.

I think `type` makes more sense than `identifier`.

[Nikita Vasilyev]

Created attachment 335488 [details]
Patch

[Matt Baker]

Comment on attachment 335488 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 335488 [details]
Patch

Clearing flags on attachment: 335488

Committed r229495: <https://trac.webkit.org/changeset/229495>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.