Bug 148484 – Web Inspector: Resources tab shouldn't show HTML resource types when inspecting JSContext

• Status: NEW
• Component: Web Inspector

[Blaze Burg]

I think we can simply hide the Resource Type selector from the sidebar if inspecting a JSContext.

[Blaze Burg]

Created attachment 259963 [details]
Screenshot of the useless UI

[Radar WebKit Bug Importer]

<rdar://problem/22440394>

[Timothy Hatcher]

It could be repurposed to filter by script type (anonymous, named, etc.)

[Blaze Burg]

Created attachment 259967 [details]
Proposed Fix

[Blaze Burg]

(In reply to comment #3)
> It could be repurposed to filter by script type (anonymous, named, etc.)

Filed: https://bugs.webkit.org/show_bug.cgi?id=148487

[Joseph Pecoraro]

Comment on attachment 259967 [details]
Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/ResourceSidebarPanel.js:44
> +        let shouldDisplayResourceTypes = WebInspector.debuggableType === WebInspector.DebuggableType.Web;

An augmented JavaScript debuggable could show non-Script resources if there is a NetworkAgent/PageAgent. So we may want to tweak this.

[Blaze Burg]

(In reply to comment #6)
> Comment on attachment 259967 [details]
> Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=259967&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/ResourceSidebarPanel.js:44
> > +        let shouldDisplayResourceTypes = WebInspector.debuggableType === WebInspector.DebuggableType.Web;
> 
> An augmented JavaScript debuggable could show non-Script resources if there
> is a NetworkAgent/PageAgent. So we may want to tweak this.

I wish there was a 1:1 between (debuggable type+protocol version) and its capabilities. Maybe we should have something more fine-grained? I want to use feature tests like ".supportsResource(type)".

There might be similar things going on in other parts of the inspector where we show irrelevant UI, such as legacy versions.

[Joseph Pecoraro]

Comment on attachment 259967 [details]
Proposed Fix

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

>>> Source/WebInspectorUI/UserInterface/Views/ResourceSidebarPanel.js:44
>>> +        let shouldDisplayResourceTypes = WebInspector.debuggableType === WebInspector.DebuggableType.Web;
>> 
>> An augmented JavaScript debuggable could show non-Script resources if there is a NetworkAgent/PageAgent. So we may want to tweak this.
> 
> I wish there was a 1:1 between (debuggable type+protocol version) and its capabilities. Maybe we should have something more fine-grained? I want to use feature tests like ".supportsResource(type)".
> 
> There might be similar things going on in other parts of the inspector where we show irrelevant UI, such as legacy versions.

r- at the moment because of this case, let me know if you want help testing this edge case.

Augmented JSContext can have any set of capabilities and we need to feature-check the Agents to then see their capabilities. A "supportsResource(type)" might be overkill because currently each backend supports the same set of resource types (see the Page.ResourceType enum). Basically here we can check, if window.PageAgent, then all resource types are possible from the backend.

There likely are some irrelevant pieces, but I've tested it a bunch in the past, we should be pretty solid right now.

[Devin Rousso]

Comment on attachment 259967 [details]
Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/ResourceSidebarPanel.js:54
> +        let shouldGroupNonExclusiveItems = shouldDisplayResourceTypes;

Is this variable only for the sake of clarity, or is there some other reason you are creating another variable that duplicates the value of shouldDisplayResourceTypes?

[Blaze Burg]

Comment on attachment 259967 [details]
Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Views/ResourceSidebarPanel.js:54
>> +        let shouldGroupNonExclusiveItems = shouldDisplayResourceTypes;
> 
> Is this variable only for the sake of clarity, or is there some other reason you are creating another variable that duplicates the value of shouldDisplayResourceTypes?

It is part of my crusade against bool positional parameters. I am having more luck in our C++ code, since we can do enum classes there.