Bug 113239 – Web Inspector: REGRESSION: Standalone image documents fail to return resource content

• Status: NEW
• Component: Web Inspector

[Timothy Hatcher]

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>

[Timothy Hatcher]

I meant Safari 6.0.3.

[pdeng6]

(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]

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]

Created attachment 195530 [details]
Patch

[Pavel Feldman]

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 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 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]

(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]

This issue still lives on in webkit trunk.

[Blaze Burg]

*** Bug 166478 has been marked as a duplicate of this bug. ***