Bug 161422

Summary: Web Inspector: XHR with text but responseType = "blob" shows blank content
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix bburg: review+

Description Joseph Pecoraro 2016-08-30 20:56:03 PDT
Summary:
XHR with text but responseType = "blob" shows blank content

Test:
<script>
let xhr = new XMLHttpRequest;
xhr.open("GET", "data.json", true);
xhr.responseType = "blob";
xhr.send();
</script>

Steps to Reproduce:
1. Inspect test
2. Reload
3. Select data.json resource
  => Content is blank, expected json content

Notes:
- XMLHttpRequest::didFinishLoad sets the content for the inspector, but it can set blank content if the XHR was loading binary data
Comment 1 Joseph Pecoraro 2016-08-30 20:56:14 PDT
<rdar://problem/28066869>
Comment 2 Joseph Pecoraro 2016-08-30 21:03:21 PDT
Created attachment 287478 [details]
[PATCH] Proposed Fix

We should really clean up our XHR <-> Inspector code. We may want to start sharing some of it with Fetch.
Comment 3 BJ Burg 2016-08-31 12:32:15 PDT
Comment on attachment 287478 [details]
[PATCH] Proposed Fix

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

r=me

So the bug is that we try to send gibberish decoded text instead of letting the inspector autodetect it or what? The changelog didn't really explain why this fix makes the frontend do anything different.

> LayoutTests/inspector/network/xhr-json-blob-has-content.html:33
> +                resource.singleFireEventListener(WebInspector.Resource.Event.LoadingDidFinish, (event) => {

The nesting here is getting excessive for my taste. I filed https://bugs.webkit.org/show_bug.cgi?id=161451 which could help.

> LayoutTests/inspector/network/xhr-json-blob-has-content.html:37
> +                        resolve();

I think it would be cleaner to remove this resolve(), then have

.then(resolve, reject)

or

.then(resolve).catch(reject)

at the end

> LayoutTests/inspector/network/xhr-json-blob-has-content.html:57
> +                        resolve();
> +                    }, reject);

Ditto
Comment 4 Joseph Pecoraro 2016-08-31 13:08:12 PDT
(In reply to comment #3)
> Comment on attachment 287478 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=287478&action=review
> 
> r=me
> 
> So the bug is that we try to send gibberish decoded text instead of letting
> the inspector autodetect it or what? The changelog didn't really explain why
> this fix makes the frontend do anything different.

Somehow this got lost in my ChangeLog. Previously we were overwriting the content with an empty string. Now we don't.
Comment 5 Joseph Pecoraro 2016-08-31 15:18:34 PDT
<https://trac.webkit.org/changeset/205268>