Implement "Copy POST Parameters" context menu item

RESOLVED FIXED in Firefox 40

Status

defect
P1
normal
RESOLVED FIXED
4 years ago
11 months ago

People

(Reporter: janx, Assigned: janx)

Tracking

(Blocks 1 bug)

unspecified
Firefox 41
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox40 fixed, firefox41 fixed)

Details

(Whiteboard: [polish-backlog][difficulty=easy])

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
Feature from Firebug's net panel. Example:

1. Head to http://codepad.org/
2. Open the Network panel
3. Click on "Submit"

Right-click on the first request item (sent as POST) should have a "Copy POST Parameters" option, that copies form data like so:

code=
lang=C
run=True
submit=Submit
(Assignee)

Updated

4 years ago
Depends on: 1158046
(Assignee)

Comment 3

4 years ago
Comment on attachment 8605872 [details] [diff] [review]
Fix "Copy URL Parameters" line separation on Windows.

Drive-by fix: In a previous patch, I forgot to use `string` instead of `params.join("\n")`, causing single-line pastes in Windows' Notepad.

Victor, please have a look.
Attachment #8605872 - Flags: review?(vporof)
(Assignee)

Comment 4

4 years ago
Comment on attachment 8605873 [details] [diff] [review]
Implement "Copy POST Data" context menu item.

One of the last context menu items before Firebug-parity.
I had to factor out some form data code.
Victor, please have a look.
Attachment #8605873 - Flags: review?(vporof)
Comment on attachment 8605872 [details] [diff] [review]
Fix "Copy URL Parameters" line separation on Windows.

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

wut
Attachment #8605872 - Flags: review?(vporof) → review+
Comment on attachment 8605873 [details] [diff] [review]
Implement "Copy POST Data" context menu item.

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

LGTM!
Attachment #8605873 - Flags: review?(vporof) → review+
(Assignee)

Comment 8

4 years ago
Tentative "checkin-needed", but the oranges look worrying. Please don't land if they persist.
Keywords: checkin-needed
(Assignee)

Comment 9

4 years ago
Not to worry, I submitted that try push twice, and the other one doesn't have oranges:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d542c5340d7a
https://hg.mozilla.org/mozilla-central/rev/c52d801739f0
https://hg.mozilla.org/mozilla-central/rev/86b65bc7f08f
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
(Assignee)

Comment 12

4 years ago
Comment on attachment 8605872 [details] [diff] [review]
Fix "Copy URL Parameters" line separation on Windows.

Please uplift this fix to Firefox40 (Aurora).

[Feature/regressing bug #]: "Copy URL Parameters" context menu added by bug 1150717 has a surprizing result on Windows (no line breaks in legacy editors like Notepad).

[User impact if declined]: "Copy URL Parameters" will continue to paste as a single line in Windows editors like Notepad.

[Describe test coverage new/current, TreeHerder]: The test only use single parameters, so the multi-line issue wasn't detected early.

[Risks and why]: No risk, it's a trivial fix.

[String/UUID change made/needed]: None.
Attachment #8605872 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 13

4 years ago
Comment on attachment 8605873 [details] [diff] [review]
Implement "Copy POST Data" context menu item.

Please uplift this new context menu item to Firefox40 (Aurora).

[Feature/regressing bug #]: None (gap from Firebug).

[User impact if declined]: Users won't be able to copy POST data using the context menu.

[Describe test coverage new/current, TreeHerder]: This uses the same test as "Copy URL Parameters", but adds a few checks for POST data.

[Risks and why]: Small. Some adjacent code was lightly refactored, and a test was modified.

[String/UUID change made/needed]: New `netmonitorUI.context.copyPostData` string + access key.
Attachment #8605873 - Flags: approval-mozilla-aurora?
I'd like to get this into aurora for June 2. As has been discussed a few times with l10n people, strings in devtools are often not translated as it is, and in this case the string in question is almost entirely technical jargon so I don't consider adding a new string here to be a huge problem.
Comment on attachment 8605872 [details] [diff] [review]
Fix "Copy URL Parameters" line separation on Windows.

Let's take it in 40. Anyway, I know how to find you in case of regression ;)
Attachment #8605872 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8605873 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
CCing Pike about the string addition.
Ugh, talking to us and getting to the same resolution doesn't seem to be the same thing, apparently.

We hard-coded the strings to English in the other devtools bugs.
(In reply to Jeff Griffiths (:canuckistani) from comment #14)
> I'd like to get this into aurora for June 2. As has been discussed a few
> times with l10n people, strings in devtools are often not translated as it
> is, and in this case the string in question is almost entirely technical
> jargon so I don't consider adding a new string here to be a huge problem.

The question should have really come before landing the strings (the bug Pike is referring to is bug 1163763).
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]

Updated

11 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.