166862 – Web Inspector: Inspecting a Main Resource that is JS/JSON does not format / syntax highlight it properly

RESOLVED FIXED 166862

Web Inspector: Inspecting a Main Resource that is JS/JSON does not format / syntax highlight it properly

https://bugs.webkit.org/show_bug.cgi?id=166862

Summary Web Inspector: Inspecting a Main Resource that is JS/JSON does not format / s...

Joseph Pecoraro

Reported 2017-01-09 14:10:34 PST

Summary: Inspecting a Main Resource that is JS/JSON does not format / syntax highlight it properly Steps to Reproduce: 1. Inspect https://svn.webkit.org/repository/webkit/trunk/Source/JavaScriptCore/features.json 2. Show the features.json main resource => Detected as JSON, is syntax highlighted, and can be pretty printed 3. Reload => Main resource is not syntax highlighted and cannot be pretty printed Notes: - Document resources may have more specific mime types than the text/html that syntheticMIMEType forces them to be.

Attachments
[PATCH] Proposed Fix (1.49 KB, patch) 2017-01-09 14:12 PST, Joseph Pecoraro	bburg: review- bburg: commit-queue-	Details Formatted Diff Diff
[Image] Behavior in trunk (325.88 KB, image/png) 2017-01-09 14:55 PST, Matt Baker	no flags	Details
[PATCH] Proposed Fix (22.08 KB, patch) 2017-01-10 17:15 PST, Joseph Pecoraro	bburg: review- buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews100 for mac-elcapitan (867.50 KB, application/zip) 2017-01-10 18:18 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews114 for mac-elcapitan (1.64 MB, application/zip) 2017-01-10 18:25 PST, Build Bot	no flags	Details
[PATCH] Proposed Fix (22.44 KB, patch) 2017-03-28 11:24 PDT, Joseph Pecoraro	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Joseph Pecoraro

Comment 1 2017-01-09 14:10:45 PST

<rdar://problem/29835977>

Joseph Pecoraro

Comment 2 2017-01-09 14:12:02 PST

Created attachment 298389 [details] [PATCH] Proposed Fix

Matt Baker

Comment 3 2017-01-09 14:55:58 PST

Created attachment 298394 [details] [Image] Behavior in trunk Following the steps to reproduce seems to produce the expected result in trunk, am I missing something?

Blaze Burg

Comment 4 2017-01-09 15:06:34 PST

Comment on attachment 298389 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=298389&action=review Code change looks fine, but this begs for a test. Try loading JSON into an iframe as the main resource. You'll want the test and the JSON file in LayoutTests/http/tests/inspector/. > Source/WebInspectorUI/ChangeLog:11 > + Don't override the mime type for Document resources if it has one. Grumpy nit, please don't fix: MIME is an initialism.

Blaze Burg

Comment 5 2017-01-09 15:57:06 PST

(In reply to comment #3) > Created attachment 298394 [details] > [Image] Behavior in trunk > > Following the steps to reproduce seems to produce the expected result in > trunk, am I missing something? Did you reload? I can reproduce on trunk.

Matt Baker

Comment 6 2017-01-09 17:35:28 PST

(In reply to comment #5) > (In reply to comment #3) > > Created attachment 298394 [details] > > [Image] Behavior in trunk > > > > Following the steps to reproduce seems to produce the expected result in > > trunk, am I missing something? > > Did you reload? I can reproduce on trunk. I hadn't tried reloading. Now I can reproduce it.

Joseph Pecoraro

Comment 7 2017-01-10 17:15:29 PST

Created attachment 298526 [details] [PATCH] Proposed Fix

Joseph Pecoraro

Comment 8 2017-01-10 17:17:13 PST

Comment on attachment 298526 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=298526&action=review > LayoutTests/http/tests/inspector/network/resource-mime-type.html:285 > + mimeType: "text/plain", // NOTE: Unsure why this MIME type is different than when XHR requested it. I'm seeing super weird behavior where we sometimes see one and sometimes the other (text/plain, application/octet-stream). Running this test multiple times encounters this issue, and also the Console Message above is finicky...

Build Bot

Comment 9 2017-01-10 18:17:56 PST

Comment on attachment 298526 [details] [PATCH] Proposed Fix Attachment 298526 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2866950 New failing tests: http/tests/inspector/network/resource-mime-type.html

Build Bot

Comment 10 2017-01-10 18:18:00 PST

Created attachment 298541 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 11 2017-01-10 18:25:26 PST

Comment on attachment 298526 [details] [PATCH] Proposed Fix Attachment 298526 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2866952 New failing tests: http/tests/inspector/network/resource-mime-type.html

Build Bot

Comment 12 2017-01-10 18:25:29 PST

Created attachment 298542 [details] Archive of layout-test-results from ews114 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Blaze Burg

Comment 13 2017-02-02 09:57:54 PST

Comment on attachment 298526 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=298526&action=review Marking r- because of EWS failures. Probably need more investigation of the flaky test case. > LayoutTests/http/tests/inspector/network/resource-mime-type.html:85 > + json: "application/octet-stream", this is surprising, i have seen things like text/json before. Can we have multiple test cases for cases like this? Or is this mime type specific to XHR requests? > LayoutTests/http/tests/inspector/network/resource-mime-type.html:99 > + mimeType: mimeTypeForExtension.js, this is clever. >> LayoutTests/http/tests/inspector/network/resource-mime-type.html:285 >> + mimeType: "text/plain", // NOTE: Unsure why this MIME type is different than when XHR requested it. > > I'm seeing super weird behavior where we sometimes see one and sometimes the other (text/plain, application/octet-stream). Running this test multiple times encounters this issue, and also the Console Message above is finicky... This seems to be the only thing blocking this patch from review/landing. If you want to land everything else separately, then investigate this issue (which seems related to the actual bug at hand here), that's fine with me.

Joseph Pecoraro

Comment 14 2017-03-28 11:24:12 PDT

Created attachment 305614 [details] [PATCH] Proposed Fix This gets rid of the flakiness by making a few minor changes: 1. Not using a Strict DOCTYPE to remove a flakey console warning message 2. Forcing application/json mime type for a JSON resource (data.json => json.php)

Blaze Burg

Comment 15 2017-03-28 13:26:27 PDT

Comment on attachment 305614 [details] [PATCH] Proposed Fix r=me

WebKit Commit Bot

Comment 16 2017-03-28 13:57:15 PDT

Comment on attachment 305614 [details] [PATCH] Proposed Fix Clearing flags on attachment: 305614 Committed r214492: <http://trac.webkit.org/changeset/214492>

WebKit Commit Bot

Comment 17 2017-03-28 13:57:18 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware All

OS All

Product WebKit

Component Web Inspector

Assignee

Joseph Pecoraro

Reported

2017-01-09 14:10 PST

Modified

2017-03-28 13:57 PDT History

CC List

8 users Show

URL

Keywords InRadar

Depends on

Blocks