Bug 166477 - Web Inspector: after inspecting an image, selecting the main resource should show an image preview
Summary: Web Inspector: after inspecting an image, selecting the main resource should ...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-12-25 14:58 PST by BJ Burg
Modified: 2017-06-09 16:09 PDT (History)
5 users (show)

See Also:


Attachments
[SCREENSHOT] (606.09 KB, image/png)
2016-12-25 14:58 PST, BJ Burg
no flags Details
Patch (8.89 KB, patch)
2017-04-08 19:06 PDT, Matt Baker
joepeck: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2016-12-25 14:58:29 PST
Created attachment 297755 [details]
[SCREENSHOT]

STEPS TO REPRODUCE:

1. Drag an image into a tab
2. Open Web Inspector
3. Go to resources tab
4. Click on main resource

=> Shows a source code text editor, not the image
Comment 1 Radar WebKit Bug Importer 2016-12-25 14:58:41 PST
<rdar://problem/29805723>
Comment 2 Matt Baker 2017-04-08 19:06:16 PDT
Created attachment 306591 [details]
Patch
Comment 3 Matt Baker 2017-04-08 19:27:59 PDT
Comment on attachment 306591 [details]
Patch

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

> Source/WebCore/inspector/InspectorNetworkAgent.cpp:383
> +    if (type != newType && (type != InspectorPageAgent::OtherResource && newType != InspectorPageAgent::DocumentResource) && newType != InspectorPageAgent::XHRResource && newType != InspectorPageAgent::OtherResource)

This feels wrong. I'm not sure what type transitions are allowed here. Will investigate.
Comment 4 Timothy Hatcher 2017-04-10 14:14:47 PDT
Comment on attachment 306591 [details]
Patch

Does this fix the double resource in the sidebar?
Comment 5 Matt Baker 2017-04-10 14:23:17 PDT
(In reply to Timothy Hatcher from comment #4)
> Comment on attachment 306591 [details]
> Patch
> 
> Does this fix the double resource in the sidebar?

The change to FrameResourceManager.js:683 prevents a child from being added that duplicates the parent frame main resource.
Comment 6 Joseph Pecoraro 2017-06-09 16:09:02 PDT
Comment on attachment 306591 [details]
Patch

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

I'm going to r- because this patch probably no longer applies.

>> Source/WebCore/inspector/InspectorNetworkAgent.cpp:383
>> +    if (type != newType && (type != InspectorPageAgent::OtherResource && newType != InspectorPageAgent::DocumentResource) && newType != InspectorPageAgent::XHRResource && newType != InspectorPageAgent::OtherResource)
> 
> This feels wrong. I'm not sure what type transitions are allowed here. Will investigate.

I agree, this is confusing. It needs some kind of justification / explanation. Perhaps writings the condition differently would make it clearer?

I think there are other places in the code that say "If this is a MainResource assume its a DocumentResource". Maybe that is the ultimate problem that should be addressed?