Closed Bug 1636804 Opened 8 months ago Closed 8 months ago

Network monitor now wrapping all values of HTTP headers with double quotes, leading to awkward escaping of values that actually contain quotes

Categories

(DevTools :: Netmonitor, defect, P2)

77 Branch
defect

Tracking

(firefox-esr68 unaffected, firefox76 unaffected, firefox77 fixed, firefox78 fixed)

RESOLVED FIXED
Firefox 78
Tracking Status
firefox-esr68 --- unaffected
firefox76 --- unaffected
firefox77 --- fixed
firefox78 --- fixed

People

(Reporter: jake, Assigned: bomsy)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Attached image parsed-vs-raw.png

All header values in the network monitor are now surrounded by double quotes, straying from how they're shown in the raw data view (and everywhere else — curl -I, Chrome devtools, etc). I'm on 77.0b3 and it seems like this also popped up and was fixed in Bug 1258628 four years ago.

I (very quickly) skimmed the diff of Bug 1613884 where it looks like this changed and based on the tests here (L62 & L64 of test/browser_net_post-data-03.js) it seems like this might be expected behavior now:

https://phabricator.services.mozilla.com/differential/changeset/?ref=2404029

...but as seen in the screenshots, this gets really awkward-looking when it comes to headers that actually have double quotes in them (in my humble opinion, at least!). ETags are a good ubiquitous example of this.

Feel free to completely disregard if this is truly intentional, it just caught me off-guard today and after finding Bug 1258628 I thought it was worth reporting. :)

Keywords: regression
Priority: -- → P3
Regressed by: 1613884

Set release status flags based on info from the regressing bug 1613884

Bomsy, could we drop the quotes and escaping altogether, as the JSON string treatment doesn't make much sense for headers where everything is a string? This would reduce dead space and simplify the UI.

P2, as this also adds " to all values copied by text selection.

Severity: -- → S3
Flags: needinfo?(hmanilla)
Priority: P3 → P2
Assignee: nobody → hmanilla
Pushed by hmanilla@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/be73bced2080
Remove quotes and escaping for header sections r=nchevobbe
Status: UNCONFIRMED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 78
Flags: needinfo?(hmanilla)

Should we nominate this for Beta approval to get it fixed in 77 before it ships to release? Please do so ASAP if yes.

Flags: needinfo?(hmanilla)

Comment on attachment 9150400 [details]
Bug 1636804 - Remove quotes and escaping for header sections r=nchevobbe

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Affects only web developers, its a simple change, just changes property values
  • User impact if declined: Affects only web developers
  • Fix Landed on Version: 77
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Simple change, changing property values
  • String or UUID changes made by this patch:
Flags: needinfo?(hmanilla)
Attachment #9150400 - Flags: approval-mozilla-esr68?

Comment on attachment 9150400 [details]
Bug 1636804 - Remove quotes and escaping for header sections r=nchevobbe

The patch looks safe, has tests and is a 77 regression. Uplift approved for beta. Please be cautious with the branch you request for uplift (you requested the es branch and it was meant for beta, almost missed it, thanks jcristau for noticing!)

Attachment #9150400 - Flags: approval-mozilla-esr68?
Attachment #9150400 - Flags: approval-mozilla-esr68-
Attachment #9150400 - Flags: approval-mozilla-beta+

Ahh .. my bad thanks jcristau and pascal

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