Bug 183317 - Web Inspector: Sources: Open all resources in Sources tab instead of Resources/Debugger
Summary: Web Inspector: Sources: Open all resources in Sources tab instead of Resource...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks: 183420
  Show dependency treegraph
 
Reported: 2018-03-03 17:19 PST by Nikita Vasilyev
Modified: 2018-03-09 17:24 PST (History)
5 users (show)

See Also:


Attachments
Patch (8.25 KB, patch)
2018-03-09 15:23 PST, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Patch (8.20 KB, patch)
2018-03-09 15:27 PST, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Patch (7.00 KB, patch)
2018-03-09 16:16 PST, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Patch (6.90 KB, patch)
2018-03-09 16:45 PST, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2018-03-03 17:19:46 PST
When Sources tab is enabled, all resources that currently open in Resources or Debugger should open in Sources tab instead.
Comment 1 Radar WebKit Bug Importer 2018-03-03 17:20:01 PST
<rdar://problem/38108455>
Comment 2 Nikita Vasilyev 2018-03-03 17:23:19 PST
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
Comment 3 Nikita Vasilyev 2018-03-09 15:23:30 PST
Created attachment 335474 [details]
Patch
Comment 4 Nikita Vasilyev 2018-03-09 15:27:02 PST
Created attachment 335476 [details]
Patch
Comment 5 Matt Baker 2018-03-09 16:02:33 PST
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`?
Comment 6 Nikita Vasilyev 2018-03-09 16:16:22 PST
Created attachment 335485 [details]
Patch
Comment 7 Matt Baker 2018-03-09 16:19:15 PST
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;
Comment 8 Devin Rousso 2018-03-09 16:25:41 PST
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`.
Comment 9 Nikita Vasilyev 2018-03-09 16:45:36 PST
Created attachment 335488 [details]
Patch
Comment 10 Matt Baker 2018-03-09 16:59:29 PST
Comment on attachment 335488 [details]
Patch

r=me
Comment 11 WebKit Commit Bot 2018-03-09 17:24:28 PST
Comment on attachment 335488 [details]
Patch

Clearing flags on attachment: 335488

Committed r229495: <https://trac.webkit.org/changeset/229495>
Comment 12 WebKit Commit Bot 2018-03-09 17:24:30 PST
All reviewed patches have been landed.  Closing bug.