Bug 166862

Summary:

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

Product:

WebKit

Reporter:

Joseph Pecoraro <joepeck>

Component:

Web Inspector

Assignee:

Joseph Pecoraro <joepeck>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, buildbot, commit-queue, inspector-bugzilla-changes, joepeck, mattbaker, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
[PATCH] Proposed Fix	bburg: review-, bburg: commit-queue-
[Image] Behavior in trunk	none
[PATCH] Proposed Fix	bburg: review-, buildbot: commit-queue-
Archive of layout-test-results from ews100 for mac-elcapitan	none
Archive of layout-test-results from ews114 for mac-elcapitan	none
[PATCH] Proposed Fix	none

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.