955933 - Allow copying the network response string

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[Dave Townsend [:mossop]]

After a JSON request I can see the JSON data in the right panel of the network monitor. Clicking a value opens a text box with the value selected. Right clicking shows a menu with the usual edit commands. Clicking copy doesn't add the value to the clipboard though.

Comment 1

[Pierre de La Morinerie]

Attachment: Allow copying the network response string.

This patch adds a "Copy Response as String" item to the network request context menu.
The item is hidden if the response is not ready yet or doesn't have a text representation.

Comment 2

[Pierre de La Morinerie]

Comment on attachment 8443939 [details] [diff] [review]
Allow copying the network response string.

Hello Victor, can you please have a look at this? Thanks!

Comment 3

[Victor Porof [:vporof][:vp]]

Comment on attachment 8443939 [details] [diff] [review]
Allow copying the network response string.

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

Looks good! Please add a test as well. They are in devtools/netmonitor/test, and browser_net_copy_url.js looks like a good one to clone.

Comment 5

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 955933.patch

Rebased and added a test case from the original patch, which implements a context menu item for copying the response.

The original problem described by mossop in comment 1 seems to be working for me since I can click copy the value from the context menu of a value's textbox in the json inspector in the network response.

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 955933.patch

Rebased browser.ini 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa5c129d9682

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/9b269f3bf1a5

Comment 8

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=2505119&repo=fx-team

Comment 9

[Francesco Lodolo [:flod]]

Why are you using an external accesskey (G), when the label is "Copy Response"?

Comment 10

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Francesco Lodolo [:flod] (UTC+1) from comment #9)
> Why are you using an external accesskey (G), when the label is "Copy
> Response"?

I mainly kept it from the original patch. What would you advise?

Comment 11

[Francesco Lodolo [:flod]]

It's an accesskey, you should use a character available in the string unless all of them are already taken.

Comment 12

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 955933.patch

Couldn't reproduce the error locally. I changed the access key for copyResponse ("R") and an existing accesskey for EditAndResend ("E", previously "R").

Comment 13

[Gabriel Luong [:gl] (ΦωΦ)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b323060e45f

Comment 14

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Gabriel Luong (:gl) from comment #13)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b323060e45f

Thanks - works really well!

Comment 15

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8588947 [details] [diff] [review]
955933.patch

@vporof: Not quite sure if this sure needs another review, but only thing that changed was the access key. I couldn't reproduce the error seen when the patch was landed in fx-team and try build is green again. Would you advise trying to land the patch again?

@flod: is there any localization issue if an accesskey was simply swapped out for another?

Comment 16

[Francesco Lodolo [:flod]]

(In reply to Gabriel Luong (:gl) from comment #15)
> @flod: is there any localization issue if an accesskey was simply swapped
> out for another?

No, you're free to swap and reorganize accesskeys as you need, each locale should pick their own accesskeys and check for conflicts.

The only border case is when you move a string into a completely different context (e.g. you move a preference from one panel to another), and that might create unexpected conflicts, but it's not what happened here.

Comment 17

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Assigning P1 priority for some devedition-40 bugs. 

Filter on '148DD49E-D162-440B-AE28-E22632FC20AC'

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/f6f3ab17d638

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Backed out for browser_net_copy_as_curl.js failures in e10s mode.
https://hg.mozilla.org/integration/fx-team/rev/a811b79720ac

4675 INFO TEST-UNEXPECTED-FAIL | browser/devtools/netmonitor/test/browser_net_copy_as_curl.js | Timed out while polling clipboard for pasted data. -
Stack trace:
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:wait:910
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForClipboard/wait/<:927
null:null:0
4676 INFO TEST-UNEXPECTED-FAIL | browser/devtools/netmonitor/test/browser_net_copy_as_curl.js | Creating a cURL command for the currently selected item was unsuccessful. -
Stack trace:
chrome://mochitests/content/browser/browser/devtools/netmonitor/test/browser_net_copy_as_curl.js:onFailure:61
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:wait:912
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForClipboard/wait/<:927
null:null:0

Comment 20

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: 955933.patch

It appears that I added the new test between the curl-test and its skip-if declaration and thus enabled the already failing curl test for e10s. 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b683bf273760

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/033d7036b755

Comment 22

[Gabriel Luong [:gl] (ΦωΦ)]

https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=033d7036b755

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/033d7036b755

Comment 24

[Peter Hozak]

This is somewhat arbitrary place from where to copy the response, I was looking for it on the Response tab. I found it in the context menu of the _Request_ line only after reading this bug :)