Closed Bug 1258628 Opened 8 years ago Closed 7 years ago

Network Monitor displays extra quotes around all HTTP header values, including already quoted values

Categories

(DevTools :: Netmonitor, defect, P2)

Product:
DevTools
Component:
Netmonitor
Type:
defect

Tracking

(firefox45 affected, firefox46 affected, firefox47 affected, firefox48 affected, firefox-esr38 affected, firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox45 --- affected
firefox46 --- affected
firefox47 --- affected
firefox48 --- affected
firefox-esr38 --- affected
firefox55 --- fixed

People

(Reporter: cpeterson, Assigned: ruturaj)

References

Details

(Whiteboard: parity-chrome,parity-ie,parity-edge)

Attachments

(1 file, 2 obsolete files)

fix-1258628-3.patch
16.42 KB, patch
ntim
: review+
Details | Diff | Splinter Review 
This is confusing when the original HTTP header values are not quoted. This is doubly-confusing for HTTP header values that are quoted. Etags values are commonly quoted, e.g.:

  Etag: "W/"P5R5TLDo""
  Etag: ""56f0fb31-7703c""

The dev tools in Chrome, Safari, Edge, and IE11 do not display quotes around HTTP header values.
Good point, agree, quotes around HTTP header values should not be displayed/appended.

Honza
Priority: -- → P2
Assignee: nobody → ruturaj
Attached patch fix-1258628-1.patch (obsolete) — — Splinter Review 
This problem persists across following Network-Monitor Detail Panels (showing confusing double quotes)
- Headers
- Cookies
- Params
- Response (parsed JSON tree)

The fix I've made solves problem for all wonder if that is OK?

- Added useQuotes: false to TreeView of properties-view.js
- Fixed following test cases
devtools/client/netmonitor/test/browser_net_complex-params.js
devtools/client/netmonitor/test/browser_net_content-type.js
devtools/client/netmonitor/test/browser_net_json-b64.js
devtools/client/netmonitor/test/browser_net_json_custom_mime.js
devtools/client/netmonitor/test/browser_net_json_text_mime.js
devtools/client/netmonitor/test/browser_net_jsonp.js
devtools/client/netmonitor/test/browser_net_post-data-01.js
devtools/client/netmonitor/test/browser_net_post-data-02.js
devtools/client/netmonitor/test/browser_net_post-data-03.js
devtools/client/netmonitor/test/browser_net_status-codes.js


Forked from: f3f86c130ef565bc1358cdf9cd35e74dbd7b6a9b
Attachment #8851196 - Flags: review?(jsnajdr)
Comment on attachment 8851196 [details] [diff] [review]
fix-1258628-1.patch

Hey Tim,

It seems :jsnajdr is unavailable. Could you please take a look at this if possible?

Thanks,
Rutu
Attachment #8851196 - Flags: review?(jsnajdr) → review?(ntim.bugs)
Thanks for working on this!

Can you please rebase?
The dir structure changed in bug 1350215
Flags: needinfo?(ruturaj)
Attached patch fix-1258628-2.patch (obsolete) — — Splinter Review 
I've updated the patch based on latest master.
Attachment #8851196 - Attachment is obsolete: true
Attachment #8851196 - Flags: review?(ntim.bugs)
Flags: needinfo?(ruturaj)
Attachment #8853918 - Flags: review?(ntim.bugs)
Comment on attachment 8853918 [details] [diff] [review]
fix-1258628-2.patch

Review of attachment 8853918 [details] [diff] [review]:
-----------------------------------------------------------------

Works great, thanks!
Attachment #8853918 - Flags: review?(ntim.bugs) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dcbdfb3fdaf2
Network monitor displays extra double quotes. r=ntim
Keywords: checkin-needed
backed out for eslint failure like https://treeherder.mozilla.org/logviewer.html#?job_id=88304201&repo=mozilla-inbound
Flags: needinfo?(ruturaj)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9647443cc6f5
Backed out changeset dcbdfb3fdaf2 for eslint failure
- fixed the eslint errors
Attachment #8853918 - Attachment is obsolete: true
Flags: needinfo?(ruturaj)
Attachment #8854007 - Flags: review?(ntim.bugs)
Attachment #8854007 - Flags: review?(ntim.bugs) → review+
Whiteboard: parity-chrome,parity-ie,parity-edge → parity-chrome,parity-ie,parity-edge,checkin-needed
Keywords: checkin-needed
Whiteboard: parity-chrome,parity-ie,parity-edge,checkin-needed → parity-chrome,parity-ie,parity-edge
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa74f9c5bf90
Network monitor displays extra double quotes. r=ntim
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fa74f9c5bf90
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Thanks Tim.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: