Bug 113239

Summary: Web Inspector: REGRESSION: Standalone image documents fail to return resource content
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: apavlov, bburg, ddkilzer, graouts, inspector-bugzilla-changes, japhet, keishi, loislo, pan.deng, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit-bug-importer, yurys
Priority: P2 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://daringfireball.net/misc/2013/03/newyorker-infographic.jpg
Attachments:
Description Flags
Patch pfeldman: review-

Timothy Hatcher
Reported 2013-03-25 14:02:59 PDT
Load http://daringfireball.net/misc/2013/03/newyorker-infographic.jpg and try to view the resource. There is a broken image — and empty data URL is shown. Happens in TOT, Chrome Canary and Chrome 25. Works in Safari 6.1.3. <rdar://problem/13498598>
Attachments
Patch (3.23 KB, patch)
2013-03-28 04:29 PDT, pdeng6
pfeldman: review-
Timothy Hatcher
Comment 1 2013-03-25 14:08:24 PDT
I meant Safari 6.0.3.
pdeng6
Comment 2 2013-03-27 01:21:31 PDT
(In reply to comment #0) > Load http://daringfireball.net/misc/2013/03/newyorker-infographic.jpg and try to view the resource. There is a broken image — and empty data URL is shown. > > Happens in TOT, Chrome Canary and Chrome 25. Works in Safari 6.1.3. > > <rdar://problem/13498598> I reproduced and investigated this issue in chromium 27.0.1452.0. When newyorker-infographic.jpg is requested alone, its CachedResource type is "MainResource", and both Inspector front-end and backend aware it as "DocumentResource"(by function InspectorPageAgent::cachedResourceType), so that inspector would have liked to present its preview as a text content but actually failed. @japhet, will the "MainResource" type be changed from CachedResource in future? or is it possible to distinguish the real type of MainResource? thanks :) Pan
pdeng6
Comment 3 2013-03-28 02:53:52 PDT
As CachedResource::MainResource may be image/document/other real type, is it possible to use mimeType in response to determine Inspector resourceType for this case? I will prepare a patch to demonstrate.
pdeng6
Comment 4 2013-03-28 04:29:28 PDT
Pavel Feldman
Comment 5 2013-03-28 04:43:45 PDT
Comment on attachment 195530 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195530&action=review > Source/WebCore/inspector/InspectorPageAgent.cpp:324 > + // Since MainResource may be actual image resource, use mime type to determine the type for Inspector's view This comment does not add much, consider removing. > Source/WebCore/inspector/InspectorPageAgent.cpp:326 > + return MIMETypeRegistry::isSupportedImageMIMEType(cachedResource.response().mimeType()) ? InspectorPageAgent::ImageResource : InspectorPageAgent::DocumentResource; Why not to check cached resource's type instead? > Source/WebCore/inspector/InspectorResourceAgent.cpp:273 > + else if (equalIgnoringFragmentIdentifier(response.url(), loader->url()) && !loader->isCommitted() && type == InspectorPageAgent::OtherResource) Why would other result in document?
pdeng6
Comment 6 2013-04-01 01:05:21 PDT
Comment on attachment 195530 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195530&action=review >> Source/WebCore/inspector/InspectorPageAgent.cpp:326 >> + return MIMETypeRegistry::isSupportedImageMIMEType(cachedResource.response().mimeType()) ? InspectorPageAgent::ImageResource : InspectorPageAgent::DocumentResource; > > Why not to check cached resource's type instead? Currently, besides document resource, standalone image/script/css... will also be requested as cachedResource::MainResource in loader, so only cachedResource::MainResource type is not enough to understand the inspector type. >> Source/WebCore/inspector/InspectorResourceAgent.cpp:273 >> + else if (equalIgnoringFragmentIdentifier(response.url(), loader->url()) && !loader->isCommitted() && type == InspectorPageAgent::OtherResource) > > Why would other result in document? "&& type == InspectorPageAgent::OtherResource" is there before https://trac.webkit.org/changeset/107672. I think without this condition, the rule is too strong, for example, http://daringfireball.net/misc/2013/03/newyorker-infographic.jpg was request alone, its documentloader url and response url will be the same, then its type will be forcefully set to DocumentResource for inspector without considering any previous prediction. thanks for your review :) Pan
Timothy Hatcher
Comment 7 2013-04-01 05:34:57 PDT
Comment on attachment 195530 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195530&action=review >>> Source/WebCore/inspector/InspectorPageAgent.cpp:326 >>> + return MIMETypeRegistry::isSupportedImageMIMEType(cachedResource.response().mimeType()) ? InspectorPageAgent::ImageResource : InspectorPageAgent::DocumentResource; >> >> Why not to check cached resource's type instead? > > Currently, besides document resource, standalone image/script/css... will also be requested as cachedResource::MainResource in loader, so only cachedResource::MainResource type is not enough to understand the inspector type. What about standalone media (video and audio) documents?
pdeng6
Comment 8 2013-04-02 02:16:24 PDT
(In reply to comment #7) > (From update of attachment 195530 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=195530&action=review > > >>> Source/WebCore/inspector/InspectorPageAgent.cpp:326 > >>> + return MIMETypeRegistry::isSupportedImageMIMEType(cachedResource.response().mimeType()) ? InspectorPageAgent::ImageResource : InspectorPageAgent::DocumentResource; > >> > >> Why not to check cached resource's type instead? > > > > Currently, besides document resource, standalone image/script/css... will also be requested as cachedResource::MainResource in loader, so only cachedResource::MainResource type is not enough to understand the inspector type. > > What about standalone media (video and audio) documents? My investigation, firstly requested as CachedResource::MainResource, then CachedResource::RawResource.
Brian Burg
Comment 9 2014-12-13 13:07:07 PST
This issue still lives on in webkit trunk.
Blaze Burg
Comment 10 2016-12-25 23:29:41 PST
*** Bug 166478 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.