Bug 141389 – Web Inspector: content views for resources loaded through XHR do not reflect declared mime-type

• Component: Web Inspector
• Status: RESOLVED FIXED

[Brian Burg]

This seems kinda screwy, since data:url resources will be placed in the Font group. But if it were loaded via XHR, then it goes to the XHR group, given a blank icon and Resource Type in the sidebar reads "XHR". The fact that it came by XHR seems secondary to whether it's HTML, image, etc.

(It does seem that they are properly syntax highlighted, so maybe the text editor respects mime type but content view creation doesn't?)

[Radar WebKit Bug Importer]

<rdar://problem/19767070>

[Brian Burg]

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

[Brian Burg]

Another repro (copied from the bug dupe'd to this one):

1. Load http://en.wikipedia.org/wiki/Deh_Kahan
2. Click on the map to show a bigger view, which is loaded by XHR
3. Select the XHR resource in the resource sidebar panel
4. Gibberish, as what appears to be binary data is interpreted as text

[Derk-Jan Hartman]

Problem still exists in Safari 9 Seed1

[Blaze Burg]

Still reproduces after other encoding issue was fixed.

[Johan K. Jensen]

It looks like the frontend could quickly get updated to use the correct mimetype, but it doesn’t look like we get the correct data from the backend.
NetworkAgent.getResponseBody (called from Resource.js) returns (a promise with) the raw image, instead of a base64 encoded blob, which is what gets shown as text.
Looks like it comes from InspectorNetworkAgent::didFinishXHRLoading and NetworkResourcesData::setResourceContent, but not quite sure how far back to track it.

[Joseph Pecoraro]

<rdar://problem/5940775>

[Joseph Pecoraro]

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

[Joseph Pecoraro]

Created attachment 328403 [details]
[PATCH] Proposed Fix

I suspect there might be some fallout here from default XHR application/octet-stream (which we used to treat as Text will now treat as binary), and probably show a blank Generic View.

After some testing I'll see if we should add back a fallback to treat this as text for XHR only (not for Other / Fetch). Ideally a binary / general fallback view would be best.

[Blaze Burg]

Comment on attachment 328403 [details]
[PATCH] Proposed Fix

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

r=me, I am so glad you finally dug into this. It's one of my biggest pet peeves.

> LayoutTests/inspector/page/searchInResources.html:27
> +    InspectorTest.debug();

Please remove.

> Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:872
> +        || MIMETypeRegistry::isSupportedNonImageMIMEType(mimeType);

I'm curious what will happen with video content and video metadata. This is something we can revisit later since we do a poor job anyway and it's not a current focus.

> Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:875
> +RefPtr<TextResourceDecoder> InspectorNetworkAgent::createTextDecoder(const String& mimeType, const String& textEncodingName)

Nit: this should return a Ref.

> Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:898
> +    if (InspectorNetworkAgent::cachedResourceContent(cachedResource, &result, &base64Encoded)) {

Eww, we should clean up the parameter types. Can you make this by reference?

> Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:906
> +bool InspectorNetworkAgent::cachedResourceContent(CachedResource& resource, String* result, bool* base64Encoded)

Ditto. Looks like there are only two use sites.

> Source/WebCore/inspector/agents/InspectorPageAgent.cpp:492
> +        if (auto* resource = cachedResource(frame, kurl)) {

kurl 👴🏻

> Source/WebInspectorUI/ChangeLog:26
> +        Better handle a no content cases.

-a

> Source/WebInspectorUI/ChangeLog:32
> +        This is done by looking at the ResourceType, and MIME Type.

-,

[Blaze Burg]

"/Volumes/Data/EWS/WebKit/Source/WebCore/inspector/agents/InspectorNetworkAgent.cpp:885:16: error: no viable conversion from 'WTF::Ref<WebCore::TextResourceDecoder>' to 'RefPtr<WebCore::TextResourceDecoder>'
        return decoder;
               ^~~~~~~
"

Looks like you need to use this constructor:

template<typename T> template<typename U> inline RefPtr<T>::RefPtr(Ref<U>&& reference)

[Joseph Pecoraro]

Created attachment 328415 [details]
[PATCH] For Bots

[EWS Watchlist]

Comment on attachment 328415 [details]
[PATCH] For Bots

Attachment 328415 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5495012

New failing tests:
http/tests/inspector/network/xhr-response-body.html
http/tests/inspector/network/fetch-response-body.html

[EWS Watchlist]

Created attachment 328424 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 328415 [details]
[PATCH] For Bots

Attachment 328415 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5495079

New failing tests:
http/tests/inspector/network/xhr-response-body.html
http/tests/inspector/network/fetch-response-body.html

[EWS Watchlist]

Created attachment 328426 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Joseph Pecoraro]

Created attachment 328432 [details]
[PATCH] For Bots (with test debugging)

[EWS Watchlist]

Comment on attachment 328415 [details]
[PATCH] For Bots

Attachment 328415 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5495689

New failing tests:
loader/go-back-cached-main-resource.html

[EWS Watchlist]

Created attachment 328434 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 328432 [details]
[PATCH] For Bots (with test debugging)

Attachment 328432 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5496147

New failing tests:
http/tests/inspector/network/xhr-response-body.html
http/tests/inspector/network/fetch-response-body.html

[EWS Watchlist]

Created attachment 328435 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 328432 [details]
[PATCH] For Bots (with test debugging)

Attachment 328432 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5496276

New failing tests:
http/tests/inspector/network/xhr-response-body.html
http/tests/inspector/network/fetch-response-body.html

[EWS Watchlist]

Created attachment 328437 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Joseph Pecoraro]

So, this fails on WebKit1 because WebKit1 appears to allow plugins to register any mime type as a non-image mime type even if it is an image mime type.

TestNetscapePlugIn (used by DumpRenderTree) registers "image/png", and WebKit adds this to the  MIMETypeRegistry::getSupportedNonImageMIMETypes. Which seems completely wrong since "image/png" is an image mime type.

I'm going to drop use of SupportedNonImageMIMETypes since it looks like it can't be trusted.

[Joseph Pecoraro]

Created attachment 328481 [details]
[PATCH] Proposed Fix

[WebKit Commit Bot]

Comment on attachment 328481 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 328481

Committed r225546: <https://trac.webkit.org/changeset/225546>