1420982 - Copy as cURL ignore PATCH data

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P3)

Description

[erdnaxeli]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171115095343

Steps to reproduce:

* go on a website doing a PATCH request
* open the Developer Tools, tab Network
* triggers the PATCH request
* right click on the line corresponding to the request, copy > copy as cURL


Actual results:

I got the curl line with the data.


Expected results:

I should have the curl line with the data. The data is available if I use copy > copy POST Data.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the report!

For anyone interested, the fix could be around these lines:
https://github.com/mozilla/gecko-dev/blob/master/devtools/client/shared/curl.js#L77
(see that PATCH doesn't seem to be supported)

Honza

Comment 2

[Abhinav Koppula]

Attachment: Bug 1420982 - Fix for copy as cURL ignoring PATCH data.

Review commit: https://reviewboard.mozilla.org/r/209168/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/209168/

Comment 3

[Abhinav Koppula]

Hi Honza,
I have created a quick patch, does this fix the issue?

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Abhinav Koppula from comment #3)
> Hi Honza,
> I have created a quick patch, does this fix the issue?

Yes, I am seeing the posted data now. This also needs a test.
I think you might extend the existing one: browser_net_curl-utils.js

Honza

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8938462 [details]
Bug 1420982 - Fix for copy as cURL ignoring PATCH data.

https://reviewboard.mozilla.org/r/209168/#review215812

Thanks Abhinav!

Only needs a test.
Honza

Comment 6

[Abhinav Koppula]

Comment on attachment 8938462 [details]
Bug 1420982 - Fix for copy as cURL ignoring PATCH data.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209168/diff/1-2/

Comment 7

[Abhinav Koppula]

Hi Honza,
I have updated my patch with the test.
But eslint is giving an error saying that i have too many nested callbacks.

mozilla-central/devtools/client/netmonitor/test/html_curl-utils.html
  105:34  error  Too many nested callbacks (4). Maximum allowed is 3.  max-nested-callbacks (eslint)

ajaxGet(url, () => {
  ajaxPost(url, () => {
    ajaxPatch(url, () => {
      ajaxMultipart(url, () => {
        submitForm();
      });
    });
  });
});

So i'm guessing that we are nesting since we want the xhrs to be fired in a specific order. 
Should we somehow ignore this eslint rule for this test?

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Abhinav Koppula from comment #7)
> Should we somehow ignore this eslint rule for this test?
Yes, you can disable it by putting:

/* eslint-disable max-nested-callbacks */

...at the top of the script (just underneath of:

/* exported performRequests */

Honza

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8938462 [details]
Bug 1420982 - Fix for copy as cURL ignoring PATCH data.

https://reviewboard.mozilla.org/r/209168/#review216142

Excellent, looks good to me!

R+

Thanks Abhinav,
Honza

Comment 10

[Abhinav Koppula]

Comment on attachment 8938462 [details]
Bug 1420982 - Fix for copy as cURL ignoring PATCH data.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209168/diff/2-3/

Comment 11

[Abhinav Koppula]

Hi Honza,
Thanks for the review.
I have fixed the eslint issue as well and have pushed to TRY
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fea42cb9d7505d8819056e1da7887c102ee0c29

Comment 12

[Abhinav Koppula]

Pushed to TRY again and TRY is green.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4555eab25b2fe9f27939543be369f8bb222b3483

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6993ed46421
Fix for copy as cURL ignoring PATCH data. r=Honza

Comment 14

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/d6993ed46421