1220758 - "Open in New Tab" on POST requests makes a GET request

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

[Don Viszneki]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20151015172900

Steps to reproduce:

(I am using Firefox 41.0.2 on Ubuntu 14.04.)

1. Go to http://codebad.com/~hdon/firefox-bug.php
2. Open Developers Tools' Network tab.
3. Click "submit" button.
4. Open context menu (right click) the Network tab's representation of the enusing HTTP request.
5. Select "Open in New Tab."


Actual results:

The HTTP method (or verb) of the HTTP request in the newly created tab is GET.


Expected results:

The HTTP method (or verb) of the HTTP request in the newly created tab should have been POST.

Comment 1

[Dan Callahan [:callahad]]

Confirmed on Nightly.

Right clicking and selecting 'Copy as cURL' produces a command-line that *does* work correctly.

This might be intentional. If it is, changing "Open in New Tab" to "Open URL in New Tab" might make the actual behavior more obvious.

Comment 2

[Don Viszneki]

(In reply to Dan Callahan [:callahad] from comment #1)
> This might be intentional. If it is, changing "Open in New Tab" to "Open URL
> in New Tab" might make the actual behavior more obvious.

That sounds good, but I would really really love to have the feature do what I thought it was going to do :)

Comment 3

[vanitom]

Hello,

For a few days, I can no longer use Firebug that had this feature. "Open in new tab" with POST.
Now that I can no longer use Firebug, it's really annoying.

When I use "Copy as cURL", the "-X POST" command is missing.
Is there a report ?

curl '[url]' -H 'Host: web.dev' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:50.0) Gecko/20100101 Firefox/50.0' -H 'Accept: */*' -H 'Accept-Language: fr,fr-FR;q=0.8,en-US;q=0.5,en;q=0.3' --compressed -H 'Referer: [url]' -H 'Content-Type: application/x-www-form-urlencoded; charset=UTF-8' -H 'X-Requested-With: XMLHttpRequest' -H 'Cookie: WEBAPP=97rglfrs4k859kql3vqj64ru83; _jsuid=3265359293; tarteaucitron=!analytics=true; _ga=GA1.2.527265735.1480502656' -H 'Connection: keep-alive' -H 'Pragma: no-cache' -H 'Cache-Control: no-cache' --data 'data%5B0%5D%5Boperation_key%5D=56ee870a-34e4-42c2-87e5-47f6c0a81d32&data%5B0%5D%5Belement_id%5D=86830&data%5B0%5D%5Bform%5D%5B0%5D%5Bname%5D=_method&data%5B0%5D%5Bform%5D%5B0%5D%5Bvalue%5D=POST&data%5B0%5D%5Bform%5D%5B1%5D%5Bname%5D=data%5BMaterialOperationItem%5D%5Bowner_id%5D&data%5B0%5D%5Bform%5D%5B1%5D%5Bvalue%5D=86830&data%5B0%5D%5Bform%5D%5B2%5D%5Bname%5D=data%5BMaterialOperationParam%5D%5Bowner_id%5D&data%5B0%5D%5Bform%5D%5B2%5D%5Bvalue%5D=86830&data%5B0%5D%5Bform%5D%5B3%5D%5Bname%5D=data%5BMaterialOperationItem%5D%5Bdate%5D&data%5B0%5D%5Bform%5D%5B3%5D%5Bvalue%5D=29%2F12%2F2016&data%5B0%5D%5Bform%5D%5B4%5D%5Bname%5D=data%5BMaterialOperationItem%5D%5Bcomment%5D&data%5B0%5D%5Bform%5D%5B4%5D%5Bvalue%5D='

Comment 5

[Sebastian Zartner [:sebo]]

Bug 1160610 suggests to add a separate option to resend the request in a new tab.

So, we have two solutions:
1. Change "Open in New Tab" to do a request using the original method.
2. Add a second option "Resend Request in New Tab" using the original method (and maybe rename "Open in New Tab" to "Open URL in New Tab").

I prefer the first option, because I believe that's what most users (and especially the Firebug users) would expect the option to do.

(In reply to vanitom from comment #3)
> When I use "Copy as cURL", the "-X POST" command is missing.
> Is there a report ?

Yes, bug 1031956.

Sebastian

Comment 6

[vanitom]

Thank you Sebatian,

I agree with you, the first solution seems to me the most logical. The same query is restarted in a new tab.

Comment 7

[Sebastian Zartner [:sebo]]

One more thing to discuss is whether the DevTools should be opened automatically on the new tab.
Firebug normally did that, but that's because its activation model is based on the same-origin principle (and not per tab like in the DevTools).

Sebastian

Comment 8

[sebastien.barre]

Thanks for looking at it guys. It was a very useful debugging feature -- 'Copy as cURL' is great but nowhere as convenient.

Comment 9

[Don Viszneki]

(In reply to sebastien.barre from comment #8)
> 'Copy as cURL' is great but nowhere as convenient.

As the OP I would like to emphasize that I love "Copy as cURL."

It is a different feature with different use cases for me.

Comment 11

[info]

Hey Guys, 

this problem is really annoying, cause no-one ever will do a serious ajax request with get vars only.

Even if the feature of modify and resend is very useful at its time, when you have console tab open, you have to do 5 clicks (rigthclick on xhr, resend, send, see answer, back to console) instead of open in new tab and simply press f5 until there is no more error. And by the way, who uses mouse to develope? Annoying.

So it really needs this resend-in-new-tab-functionality in console tab AND it has to include post vars. 
Get-only will surely help noone and if, he can simply copy the address and paste it. but why should you want to loose the POST data on purpose?

I think this is a major problem, Firefox is not a useful webdevelopement machine without it any more...
Sad.

Comment 12

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

Here are some instructions for anyone interested in this bug.

1) Clean STR are in comment #0

2) The current implementation of "Open in New Tab." is here:
https://dxr.mozilla.org/mozilla-central/rev/d0462b0948e0b1147dcce615bddcc46379bdadb2/devtools/client/netmonitor/request-list-context-menu.js#189

3) Firebug's implementation has correct behavior (uses POST when appropriate) and you can see it here:
https://github.com/firebug/firebug/blob/master/extension/content/firebug/chrome/window.js#L120

So, let's clone Firebug implementation and see if it helps!

Honza

Comment 13

[guohao.yan]

Hi, may I work on this bug? This will be my GFB for school assignment.

Comment 14

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

(In reply to guohao.yan from comment #13)
> Hi, may I work on this bug? This will be my GFB for school assignment.
Sure, assigned to you!

Honza

Comment 15

[Don Viszneki]

Thank you :)

This feature is very important to me!

Comment 16

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

Hi Don, 
any progress on this?
Do you need help?

Honza

Comment 17

[Don Viszneki]

The bug is still present in Firefox 51.0.1 on Xenial :(

Comment 18

[Sebastian Zartner [:sebo]]

(In reply to Don Viszneki from comment #17)
> The bug is still present in Firefox 51.0.1 on Xenial :(

Of course, there was no patch applied yet. A bug is fixed when its status is FIXED. Also, when that issue is fixed, it will not be in version 51, but the earliest version this could be in is 54. See https://wiki.mozilla.org/RapidRelease/Calendar.

Sebastian

Comment 19

[Don Viszneki]

(In reply to Sebastian Zartner [:sebo] from comment #18)
> Of course

Sorry, I didn't quite know what to make of Honza's question :3

Comment 20

[guohao.yan]

(In reply to Jan Honza Odvarko [:Honza] from comment #14)
> (In reply to guohao.yan from comment #13)
> > Hi, may I work on this bug? This will be my GFB for school assignment.
> Sure, assigned to you!
> 
> Honza

Hi, I agreed and followed your instructions to clone the correct implementation, but I got stuck because I'm not sure how to open a new tab with the postText data. Could you help me with which functions to call? or which file would I be looking into for this particular tab opening functions.

Also, it's very difficult to find the window.open() method declaration.

Thanks, it will help a lot.

Comment 21

[awstam]

yeah this "feature" made me switch to chrome. but chrome dev tools are more focused on "speed" instead of the long road before you have to start worring bout speed. came back to firefox. but since firebug isnt there any more stuck using the dev tools. this lack of open-in-new-tab-but-keep-post thing is really making my life difficult at the moment :(

Comment 22

[[e-mail]]

> Hi, I agreed and followed your instructions to clone the correct
> implementation, but I got stuck because I'm not sure how to open a new tab
> with the postText data. Could you help me with which functions to call? or
> which file would I be looking into for this particular tab opening functions.
> 
> Also, it's very difficult to find the window.open() method declaration.
> 
> Thanks, it will help a lot.

Guohao, it will be nice if you can try with the normal web way and see if that works
http://taswar.zeytinsoft.com/javascript-http-post-data-to-new-window-or-pop-up/

By the way, you can pass postData as the 4th parameter in win.openUILinkIn https://developer.mozilla.org/en-US/docs/Archive/Add-ons/Tabbed_browser
Though I just found `win.openUILinkIn` is the firefox specific api which we want to removed them completely from netmonitor.

Comment 23

[guohao.yan]

(In reply to Fred Lin [:gasolin] from comment #22)
> > Hi, I agreed and followed your instructions to clone the correct
> > implementation, but I got stuck because I'm not sure how to open a new tab
> > with the postText data. Could you help me with which functions to call? or
> > which file would I be looking into for this particular tab opening functions.
> > 
> > Also, it's very difficult to find the window.open() method declaration.
> > 
> > Thanks, it will help a lot.
> 
> Guohao, it will be nice if you can try with the normal web way and see if
> that works
> http://taswar.zeytinsoft.com/javascript-http-post-data-to-new-window-or-pop-
> up/
> 
> By the way, you can pass postData as the 4th parameter in win.openUILinkIn
> https://developer.mozilla.org/en-US/docs/Archive/Add-ons/Tabbed_browser
> Though I just found `win.openUILinkIn` is the firefox specific api which we
> want to removed them completely from netmonitor.

Hi, thanks for the help
would it be ok to use win.gBrowser.addTab method?
I tried this by passing {postData: this.selectedItem.requestPostData.postData}, however in the browser console I'm seeing Exceptions saying no method named " available" from nsIInputStream:avaliable


here's my code:
    let win = Services.wm.getMostRecentWindow(gDevTools.chromeWindowType);
    win.selectedTab = win.getBrowser.addTab(this.selectedItem.url, {postData:                
        this.selectedItem.requestPostData.postData, relatedToCurrent: true});

am I doing something wrong here?

Comment 24

[[e-mail]]

Guohao thanks for trying, but please focus on use normal web way instead of gBrowser. 
In netmonitor we almost removed all chrome-privileged API usage and will not accept gBrowser usage anymore.

Comment 25

[guohao.yan]

Hi,

I tried to use the normal web way to open the tab with post request.
that is use window.open(url), then submit the form I created using document.createElement("form") just like the example provided.

I have 2 questions that really stops me

1. In my settings, I checked always open new tab instead of window, but window.open() would always open a new window for me in this case.

2. when I do form.submit()  console will throw submit is not a function error message, I tried to use HTMLFormElement.prototype.submit.call(form)  it would say form does not implement the interface....

I think I'm close to finish this bug, but these 2 questions are stopping me, I spent couple hours researching for answer, no result :) Could you give me a hint? Thanks

Here is a snippet of code I wrote:


 openRequestInTab() {
     let url = this.selectedItem.url;
     let win = Services.wm.getMostRecentWindow(gDevTools.chromeWindowType);
     if (this.selectedItem.requestPostData) 
     {
         var document = win.document;
         if (!document.body) 
         {
            var newBody = document.createElement("body");
            document.body = newBody;
         }
         let postData = this.selectedItem.requestPostData.postData;
         // Create hidden form object to send post request.
         var form = document.createElement('form');
         form.setAttribute("method", "POST");
         form.setAttribute("action", url);

         for (var i in postData) 
         {
             if (postData.hasOwnProperty(i)) 
             {
                 var input = document.createElement('input');
                 input.setAttribute('type', 'hidden');
                 input.setAttribute('name',  i);
                 input.setAttribute('value', postData[i]);
                 form.appendChild(input);
             }
         }

         document.body.appendChild(form);
         
         win.open(url); // don't use gBrowser to open new tab

         form.submit();
         //HTMLFormElement.prototype.submit.call(form);
         form.remove();
     }
     else {
        win.open(url);
     }
  },

Comment 26

[[e-mail]]

Guohao thanks for investigation. I saw window.open create a new window as well.
And if you print form element in console, you could find `form.submit() function not found` is because the `form` element here is a XULElelment, not HTMLElement....


Honza, though we have shim for Service.wm.getMostRecentWindow and openUILinkIn shim, but they don't support postdata and in Firefox we seems still run XUL version instead of the shim. Do you have any suggestion?

Comment 27

[awstam]

sorry to bother. im presuming this bug wont be "fixed" any time soon? earliest it can make it is 61? (2018) any work-around for it in the mean time?

Comment 28

[info]

Hi Folks. Is this the same bug or related: 
If you have a POST-tab and you press reload, it will ask to resend and resend POST data.
But open the source view of the POST tab, the reload will not ask and reloads using GET.
... bug or feature? i think its first.

Comment 29

[Sebastian Zartner [:sebo]]

(In reply to awstam from comment #27)
> sorry to bother. im presuming this bug wont be "fixed" any time soon?
> earliest it can make it is 61? (2018) any work-around for it in the mean
> time?

Earliest version will be 55 (which is currently Nightly), released in August this year.[1]

(In reply to info from comment #28)
> Hi Folks. Is this the same bug or related: 
> If you have a POST-tab and you press reload, it will ask to resend and
> resend POST data.
> But open the source view of the POST tab, the reload will not ask and
> reloads using GET.
> ... bug or feature? i think its first.

That's unrelated to this issue. It sounds like a bug, but it might be intended behavior. Please create a new bug report for it.

Sebastian

[1] https://wiki.mozilla.org/RapidRelease/Calendar

Comment 31

[[e-mail]]

In bug 1337295 comment 3 (from Vincent MONIER)

Currently, you can open a request inside a FF tab when you do "right click / Open in a new tab", but this changes POST into GET. I think it's non-intuitive and is a defect.

"Select the POST column and click `Edit and Resend`" is not easy to use in some cases, like:
• If the response has HTML & javascript, JS won't exec in the dev tab (while a normal FF tab would execute it)
• If it uses XML+XSL, devtab will only show XML version (while a FF tab would apply the XSL)
• It's a pain to do "edit & resend", check if the server responds what you wish, correct the server side otherwise, and redo a cycle again (while a FF tab would make it easy by just pressing F5)
• It won't comply with developer plugins, like GreaseMonkey/Stylish (I use them for some stuff like turning some things into links that open in NetBeans)


Hence, I think that a dev tools request should be resendable inside a Firefox tab instead. So, if the request was a GET, a tab is opened to that URL. But if it was a POST, a tab is opened to that URL doing a POST request (so maybe with the "Are you sure you want to resend these information?" alert box).

Comment 32

[[e-mail]]

After discussion with Honza, we decide to rephase `Open in a new tab` to `Resend`, and reuse the existing edit & resend action. 

So we just need to call things like `dispatch(action.resendRequest)` (need implement) in context menu to resend this request.

First, you need create an constant `RESEND_REQUEST` in constant.js,
Then create an action in 
http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/actions/requests.js#48
as `resendRequest`, to reuse exactly the same code in sendCustomRequest function

And not doing anything in
http://searchfox.org/mozilla-central/source/devtools/client/netmonitor/reducers/requests.js
(Which RECUSTOM_REQUEST did close the current request)

Guohao, would you like to try that?

Comment 33

[Don Viszneki]

(In reply to Fred Lin [:gasolin] from comment #32)
> After discussion with Honza, we decide to rephase `Open in a new tab` to
> `Resend`, and reuse the existing edit & resend action. 
> 
> So we just need to call things like `dispatch(action.resendRequest)` (need
> implement) in context menu to resend this request.

Hi Fred,

Should I file a separate bug for "Open in a new tab" feature?

Comment 34

[[e-mail]]

Don, we'd like replace the "Open in a new tab` feature to `Resend`, they will both send a new request(GET/POST, or any thing). 
The difference is `Resend` won't open a new tab. 

Does it sounds fine to you?

Comment 35

[info]

(In reply to Fred Lin [:gasolin] from comment #32)
> After discussion with Honza, we decide to rephase `Open in a new tab` to
> `Resend`, and reuse the existing edit & resend action. 
> 
> So we just need to call things like `dispatch(action.resendRequest)` (need
> implement) in context menu to resend this request.

...but this was not the point! this feature already exists. why ever.


(In reply to Fred Lin [:gasolin] from comment #34)
> The difference is `Resend` won't open a new tab. 

..that's it.

its a very big difference between resending and see the source code only or having a new tab where you can see rendered html or images or dynamically created svg!?

and F5 still will not work there. or Strg+F5. or element inspector. or anything else.

we need a new tab!

;-)

Comment 36

[Don Viszneki]

(In reply to Don Viszneki from comment #33)
> (In reply to Fred Lin [:gasolin] from comment #32)
> > After discussion with Honza, we decide to rephase `Open in a new tab` to
> > `Resend`, and reuse the existing edit & resend action. 
> > 
> > So we just need to call things like `dispatch(action.resendRequest)` (need
> > implement) in context menu to resend this request.
> 
> Hi Fred,
> 
> Should I file a separate bug for "Open in a new tab" feature?

Some use cases will be satisfied by either such feature, but some will only be satisfied by "Open in new tab" :)

Comment 37

[Don Viszneki]

(In reply to Fred Lin [:gasolin] from comment #34)
> Don, we'd like replace the "Open in a new tab` feature to `Resend`, they
> will both send a new request(GET/POST, or any thing). 
> The difference is `Resend` won't open a new tab. 
> 
> Does it sounds fine to you?

Whoops, I meant to reply to this post. Sorry!

Comment 38

[guohao.yan]

Hi,

Fred, about my approach (comment 25) to the problem that followed 'the normal web way' as you mentioned in comment 22.

There are 2 issues present, 1. being unable to open in new tab,  2. unable to create HTMLElement version of form to submit postdata.

Do you have suggestions to work around those issues so we can solve the problem?

Comment 39

[[e-mail]]

Thanks for Don and info's feedback, we should keep the current "Open in a new tab` feature since it has its position.

Therefore we inevitably have to use privileged APIs as firebug do to deal with the new tab.

Honza, could you help answer the XUL related question guohao has mentioned in comment 23?

Comment 40

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

(In reply to guohao.yan from comment #23)
> would it be ok to use win.gBrowser.addTab method?
So, yes (just like the previous comment says)

> I tried this by passing {postData:
> this.selectedItem.requestPostData.postData}, however in the browser console
> I'm seeing Exceptions saying no method named " available" from
> nsIInputStream:avaliable
> 
> 
> here's my code:
>     let win = Services.wm.getMostRecentWindow(gDevTools.chromeWindowType);
>     win.selectedTab = win.getBrowser.addTab(this.selectedItem.url,
> {postData:                
>         this.selectedItem.requestPostData.postData, relatedToCurrent: true});
> 
> am I doing something wrong here?
Does it help if you pass args into the function just like Firebug does?
(i.e. postData as 4th argument)

Here is the doc:
https://developer.mozilla.org/pl/docs/Mozilla/Tech/XUL/Metoda/addTab


Honza

Comment 41

[guohao.yan]

(In reply to Jan Honza Odvarko [:Honza] from comment #40)
> (In reply to guohao.yan from comment #23)
> > would it be ok to use win.gBrowser.addTab method?
> So, yes (just like the previous comment says)
> 
> > I tried this by passing {postData:
> > this.selectedItem.requestPostData.postData}, however in the browser console
> > I'm seeing Exceptions saying no method named " available" from
> > nsIInputStream:avaliable
> > 
> > 
> > here's my code:
> >     let win = Services.wm.getMostRecentWindow(gDevTools.chromeWindowType);
> >     win.selectedTab = win.getBrowser.addTab(this.selectedItem.url,
> > {postData:                
> >         this.selectedItem.requestPostData.postData, relatedToCurrent: true});
> > 
> > am I doing something wrong here?
> Does it help if you pass args into the function just like Firebug does?
> (i.e. postData as 4th argument)
> 
> Here is the doc:
> https://developer.mozilla.org/pl/docs/Mozilla/Tech/XUL/Metoda/addTab
> 
> 
> Honza

OK, I tried gBrowser.addTab

   openRequestInTab() {
     let win = Services.wm.getMostRecentWindow(gDevTools.chromeWindowType);
     let postData = this.selectedRequest.requestPostData.postData;
     win.selectedTab = win.gBrowser.addTab(this.selectedRequest.url, null, null, postData);
   },


Still getting the same exception:
[Exception... "JavaScript component does not have a method named: "available"'JavaScript component does not have a method named: "available"' when calling method: [nsIInputStream::available]"  nsresult: "0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)"  location: "JS frame :: resource://gre/modules/sessionstore/Utils.jsm :: serializeInputStream :: line 64"  data: no]

I don't quite understand what it means, I checked in console that postData is an object {text: "submit=submit"}

Comment 42

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

This is because you are passing an Object (`postData`) into the addTab() method. You need to create an input stream from the text (`postData.text`) and pass it instead.

This is what Firebug is doing:

let postText = this.selectedRequest.requestPostData.postText.text;
var stringStream = Http.getInputStreamFromString(postText);
postData = Cc["@mozilla.org/network/mime-input-stream;1"].createInstance(Ci.nsIMIMEInputStream);
postData.addHeader("Content-Type", "application/x-www-form-urlencoded");
postData.addContentLength = true;
postData.setData(stringStream);

See here:
https://github.com/firebug/firebug/blob/master/extension/content/firebug/chrome/window.js#L128

(of course, you need to copy also Http.getInputStreamFromString() helper)



Btw. don't forget to need-info me to make sure I have this in my TODO list.

Honza

Comment 43

[guohao.yan]

Attachment: bug1220758

Hi, Thanks for all the help, I've created the patch file in attachments.

Comment 44

[Tim Nguyen :ntim]

Comment on attachment 8854514 [details] [diff] [review]
bug1220758

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

::: devtools/client/netmonitor/src/request-list-context-menu.js
@@ +363,5 @@
> +/*
> +Helper function from https://github.com/firebug/firebug/blob/master/extension/content/firebug/lib/http.js
> +*/
> +var getInputStreamFromString = function(dataString)
> +{

You might want to fix eslint errors from ./mach eslint devtools/client/netmonitor/ --fix

@@ +370,5 @@
> +
> +    if ("data" in stringStream) // Gecko 1.9 or newer
> +        stringStream.data = dataString;
> +    else // 1.8 or older
> +        stringStream.setData(dataString, dataString.length);

No need to support Gecko 1.8 and older, this can be simplified to:

stringStream.data = dataString;

(without the if else)

Comment 45

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

Comment on attachment 8854514 [details] [diff] [review]
bug1220758

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

Thanks, the patch works for me on my machine!

Please resolve my inline comments (and ntim's comments)

Honza

::: devtools/client/netmonitor/src/request-list-context-menu.js
@@ +191,5 @@
>     * Opens selected item in a new tab.
>     */
> +   openRequestInTab() {
> +     let win = Services.wm.getMostRecentWindow(gDevTools.chromeWindowType);
> +     let postText = this.selectedRequest.requestPostData.postData.text;

this.selectedRequest.requestPostData might be null if the user right clicks on GET request.

@@ +195,5 @@
> +     let postText = this.selectedRequest.requestPostData.postData.text;
> +     var postData = null;
> +     if (postText) {
> +         var stringStream = getInputStreamFromString(postText);
> +         postData = Cc["@mozilla.org/network/mime-input-stream;1"].createInstance(Ci.nsIMIMEInputStream);

This line is too long (max length is 90)

@@ +200,5 @@
> +         postData.addHeader("Content-Type", "application/x-www-form-urlencoded");
> +         postData.addContentLength = true;
> +         postData.setData(stringStream);
> +     }
> +     win.selectedTab = win.gBrowser.addTab(this.selectedRequest.url, null, null, postData);

This line is also too long.

@@ +360,3 @@
>      };
>    }
>  };

Put one empty line after the end of RequestListContextMenu object definition.

@@ +361,5 @@
>    }
>  };
> +/*
> +Helper function from https://github.com/firebug/firebug/blob/master/extension/content/firebug/lib/http.js
> +*/

The comment should be structured as foollows:

/*
 * Helper...
 */

Also please split the comment to multiple lines (it's over 90 chars).

Comment 46

[[e-mail]]

Instead of patch netmonitor/src/request-list-context-menu.js directly,
I suggest create a new utility `netmonitor/src/utils/context-menu.js` which contains the new `openRequestInTab` function and all related helper. Then import that function into `request-list-context-menu.js`.

With that change it will be simpler to run on browser tab without chrome privilege. (just swap `/utils/context-menu.js` to a mockup with webpack alias)

Comment 47

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

(In reply to Fred Lin [:gasolin] from comment #46)
> Instead of patch netmonitor/src/request-list-context-menu.js directly,
> I suggest create a new utility `netmonitor/src/utils/context-menu.js` which
> contains the new `openRequestInTab` function and all related helper. Then
> import that function into `request-list-context-menu.js`.
> 
> With that change it will be simpler to run on browser tab without chrome
> privilege. (just swap `/utils/context-menu.js` to a mockup with webpack
> alias)
Yep, I like the idea.

What if we mimic dir structure which is there for DevTools...

For example:
devtools/share/platform/chrome/clipboard.js
alias:
devtools/share/platform/content/clipboard.js

We could do:
devtools/client/netmonitor/platform/chrome/open-request-in-tab.js
alias:
devtools/client/netmonitor/platform/content/open-request-in-tab.js

(I am not sure if we want to use context-menu.js as the file name)

@Ricky, what do you think?

Honza

Comment 48

[Tim Nguyen :ntim]

It doesn't really make sense to have a platform directory, since platform stuff is supposed to belong on the actor side (devtools/server).

Comment 49

[guohao.yan]

Attachment: adding revised patch file

Comment 50

[guohao.yan]

sorry.....something went wrong please ignore previous patch file...

Comment 51

[guohao.yan]

Attachment: bug1220758.patch

Comment 52

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

(In reply to Tim Nguyen :ntim from comment #48)
> It doesn't really make sense to have a platform directory, since platform
> stuff is supposed to belong on the actor side (devtools/server).
We might want to use different names but, apparently there is UI stuff that depends on the platform/context (Toolbox/Launchpad).

Honza

Comment 53

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

Comment on attachment 8855313 [details] [diff] [review]
bug1220758.patch

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

Thanks for new version!

Please resolve also ntim's comments (see comment #44)

Honza

::: devtools/client/netmonitor/src/request-list-context-menu.js
@@ +371,3 @@
>      };
>    }
>  };

nit: Please insert new line here

Comment 54

[[e-mail]]

How about put chrome privileged utils into `utils/chrome/`, such as `utils/chrome/open-request-in-tab.js`?

There will be not many things left there, but the structure will be clear.

Comment 55

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

(In reply to Fred Lin [:gasolin] from comment #54)
> How about put chrome privileged utils into `utils/chrome/`, such as
> `utils/chrome/open-request-in-tab.js`?
> 
> There will be not many things left there, but the structure will be clear.
Yep, I like it.

Honza

Comment 56

[guohao.yan]

Attachment: revised version of the fix, added newline and removed old version support.

Not sure about the decision of moving this into utli/chrome/ is it confirmed and I should be doing it?

Comment 57

[guohao.yan]

Comment on attachment 8855564 [details] [diff] [review]
revised version of the fix, added newline and removed old version support.

uhh sorry forgot to hg commit again ..

Comment 58

[guohao.yan]

Attachment: bug1220758.patch

Comment 59

[[e-mail]]

guohao, you've submit 2 patches, can you merge them into 1?


To moving this into `utils/chrome/`, you should also modify `utils/moz.build` to include `chrome` folder, and add a new `moz.build` file inside of `chrome` folder to let the `mach build` command knows where these files are.

Its fine to not move the function now. We need file a followup bug to make it runable on the browser tab anyway.

Comment 60

[Ricky Chien [:rickychien] (inactive)]

(In reply to Jan Honza Odvarko [:Honza] from comment #47)
> (In reply to Fred Lin [:gasolin] from comment #46)
> > Instead of patch netmonitor/src/request-list-context-menu.js directly,
> > I suggest create a new utility `netmonitor/src/utils/context-menu.js` which
> > contains the new `openRequestInTab` function and all related helper. Then
> > import that function into `request-list-context-menu.js`.
> > 
> > With that change it will be simpler to run on browser tab without chrome
> > privilege. (just swap `/utils/context-menu.js` to a mockup with webpack
> > alias)
> Yep, I like the idea.
> 
> What if we mimic dir structure which is there for DevTools...
> 
> For example:
> devtools/share/platform/chrome/clipboard.js
> alias:
> devtools/share/platform/content/clipboard.js
> 
> We could do:
> devtools/client/netmonitor/platform/chrome/open-request-in-tab.js
> alias:
> devtools/client/netmonitor/platform/content/open-request-in-tab.js
> 
> (I am not sure if we want to use context-menu.js as the file name)
> 
> @Ricky, what do you think?

Yep, I'm in favor of this pattern for supporting privilege APIs. We have done a huge effort to get rid of all privilege APIs and embrace Web standard APIs in order to run network monitor on devtools-launchpad. Any privilege APIs is not going to land in netmonitor and especially under netmonitor/src/ folder.

I don't know is that possible to support gBrowser in devtools-modules. If not, we should consider that to make "Open in New Tab" feature to be enabled only in firefox panel but disable in devtools-launchpad.

Comment 61

[Ricky Chien [:rickychien] (inactive)]

If it's possible to support open-request-in-tab.js with web standard APIs, we can go:

  * devtools/share/platform/chrome/open-request-in-tab.js

  * devtools/share/platform/content/open-request-in-tab.js

If not, then we go:

  * devtools/share/open-request-in-tab.js

And then use a feature detection for "open request in tab" in request-list-context-menu.js to make "Open in New Tab" menu visible when running on Firefox devtools.

Comment 62

[[e-mail]]

As offline discussion, we agree to put non-Web-APIs(for this case, open-request-in-tab.js) into `netmonitor/src/utils/chrome/`.

Comment 63

[Ricky Chien [:rickychien] (inactive)]

(In reply to Fred Lin [:gasolin] from comment #62)
> As offline discussion, we agree to put non-Web-APIs(for this case,
> open-request-in-tab.js) into `netmonitor/src/utils/chrome/`.

In the future, our architecture will support both google Chrome and Firefox. I'd suggest `utils/firefox` as the folder name to put any Firefox specific privilege stuff.

Comment 64

[guohao.yan]

Attachment: bug1220758.patch

Combined the 2 patch files.

Comment 65

[Tim Nguyen :ntim]

Since we're talking about chrome usage, there is actually an easy (non-chrome) way to open a tab with post data: 

Here's what I'm using for my devtools-prototyper add-on to submit post data to JSFiddle.
https://github.com/nt1m/devtools-prototyper/blob/master/src/chrome/backend/request.js#L14-L30

One issue is that this *needs* to run on server side (webpage) in order to work correctly, if you run it on the client (devtools toolbox) side, the page will open in a new window.

Comment 66

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

Comment on attachment 8856139 [details] [diff] [review]
bug1220758.patch

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

Thanks for the update!

The logic in the patch looks good we just need to move the openRequestInTab() and the helper getInputStreamFromString() into an independent module. The module should be located in the following directory:

`devtools/client/netmonitor/src/utils/firefox/open-request-in-tab.js`

This module should export `openRequestInTab()` method and required in `request-list-context-menu.js` file.

Honza

Comment 67

[Sebastian Zartner [:sebo]]

Guohao, are you still working on this patch?

Sebastian

Comment 68

[Jan Kopidol]

+1
This is really annoying bug. 
Is there at least another possibility to view POST request in new tab (plugin maybe?).
In v56 I am able to use Firebug, which has this feature for YEARS. But Firefox Developer Edition (57) won't allow installation at all (even from file).
Opening response in new tab without POST data is just useless :(.

Comment 69

[Chalda]

+1
I'm using Firefox 57.0b2 and this bug is still present.

Comment 70

[info]

+1
come on guys... cant be as hard...?
hate to use chrome for developement...
but firefox is now completely useless.

Comment 71

[zeleniy.spb]

+1

Comment 72

[Chirag D]

This is such an annoying bug. The feature was so awesome on firebug and I really need it.
It just worked well on firebug and is one of the features I used all the time.
The resource tab for network monitor on 57.0b6 is so tiny to use and very fiddly.
I would really appreciate it if this feature was fixed.

Comment 73

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

@guohao: are you planning to finish this?
Do you need any help or should I do it?

Honza

Comment 74

[Chalda]

@guohao / @Honza, are you working on it?
Thanks

Comment 75

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

Attachment: Bug 1220758 - Properly pass postData into new tab;

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

Comment 76

[Tim Nguyen :ntim]

Comment on attachment 8932116 [details]
Bug 1220758 - Properly pass postData into new tab;

https://reviewboard.mozilla.org/r/203152/#review208600

Have you tried using what's mentioned in comment 65 ?

Comment 77

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

(In reply to Tim Nguyen :ntim from comment #76)
> Comment on attachment 8932116 [details]
> Bug 1220758 - Properly pass postData into new tab;
> 
> https://reviewboard.mozilla.org/r/203152/#review208600
> 
> Have you tried using what's mentioned in comment 65 ?
No, I was discouraged by:

"One issue is that this *needs* to run on server side (webpage) in order to work correctly"

Can you provide more details?

Honza

Comment 78

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8932116 [details]
Bug 1220758 - Properly pass postData into new tab;

https://reviewboard.mozilla.org/r/203152/#review208818

I'm seeing TypeError: request.requestPostData is undefined when visiting www.google.com and randonly select / open a request with `open in new tab`.

I'm fine with using win.gBrowser.addTab for opening new tab. Although approach from comment 65 looks good, I'm afraid that opening page in a new window doesn't fit our use case.

On the other hand, we should ensure every fix will not break launchpad. Launchpad threw `Error: Cannot find module "chrome"` error since you don't add webpack alias. In order to make open request in new tab feature works for launchpad, maybe we can go back to use the approach provided from comment 65.

Comment 79

[Tim Nguyen :ntim]

(In reply to Jan Honza Odvarko [:Honza] from comment #77)
> (In reply to Tim Nguyen :ntim from comment #76)
> > Comment on attachment 8932116 [details]
> > Bug 1220758 - Properly pass postData into new tab;
> > 
> > https://reviewboard.mozilla.org/r/203152/#review208600
> > 
> > Have you tried using what's mentioned in comment 65 ?
> No, I was discouraged by:
> 
> "One issue is that this *needs* to run on server side (webpage) in order to
> work correctly"
> 
> Can you provide more details?

If you run it on the toolbox side, it will perform the post request in a new window, but if you run it on the page itself, it will properly open in a new tab.

The code should work fine in launchpad though: it will properly open in a new tab there.

Comment 80

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

Comment on attachment 8932116 [details]
Bug 1220758 - Properly pass postData into new tab;

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

Comment 81

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

(In reply to Ricky Chien [:rickychien] from comment #78)
> I'm seeing TypeError: request.requestPostData is undefined when visiting
> www.google.com and randonly select / open a request with `open in new tab`.
Should be fixed

> I'm fine with using win.gBrowser.addTab for opening new tab. Although
> approach from comment 65 looks good, I'm afraid that opening page in a new
> window doesn't fit our use case.
Agree, I am using `win.gBrowser.addTab` for the Toolbox and ntim's approach
for the Launchpad (webpack.config updated)

Ntim I used only the 'submitform' part, how does the look to you?

Honza

Comment 82

[Tim Nguyen :ntim]

Comment on attachment 8932116 [details]
Bug 1220758 - Properly pass postData into new tab;

https://reviewboard.mozilla.org/r/203152/#review209042

::: devtools/client/netmonitor/src/utils/open-request-in-tab.js:19
(Diff revision 2)
> +    data: request.requestPostData ? request.requestPostData.postData : null,
> +  });
> +}
> +
> +function openRequestInTabHelper({url, method, data}) {
> +  method = method || "POST";

Is this useful for the netmonitor ? Is there a case where we don't have the request method ?

Also, do we want to go through all the trouble of submitting a form for a get request ?

::: devtools/client/netmonitor/src/utils/open-request-in-tab.js:23
(Diff revision 2)
> +    if (typeof data[key] === "function") {
> +      copy[key] = data[key]();
> +    } else {

I don't think this part is relevant to the netmonitor.
You should just be able to use `data` directly without needing the `copy`.

::: devtools/client/netmonitor/src/utils/open-request-in-tab.js:33
(Diff revision 2)
> +  }
> +
> +  let form = document.createElement("form");
> +  form.target = "_blank";
> +  form.action = url;
> +  form.method = method;

form.method only accepts "get" and "post", so you'll probably need some code to handle "options"/"delete"/"patch" methods.

::: devtools/client/netmonitor/src/utils/open-request-in-tab.js:46
(Diff revision 2)
> +
> +  form.hidden = true;
> +  document.body.appendChild(form);
> +  form.submit();
> +  form.remove();
> +  return Promise.resolve(true);

No need to return a promise here.

Comment 83

[Tim Nguyen :ntim]

Added some comments

Comment 84

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8932116 [details]
Bug 1220758 - Properly pass postData into new tab;

https://reviewboard.mozilla.org/r/203152/#review209246

Please address what ntim suggested.

::: devtools/client/netmonitor/webpack.config.js:89
(Diff revision 2)
>        "devtools/client/sourceeditor/editor": "devtools-source-editor/src/source-editor",
>  
>        "devtools/shared/old-event-emitter": "devtools-modules/src/utils/event-emitter",
>        "devtools/shared/fronts/timeline": path.join(__dirname, "../../client/shared/webpack/shims/fronts-timeline-shim"),
>        "devtools/shared/platform/clipboard": path.join(__dirname, "../../client/shared/webpack/shims/platform-clipboard-stub"),
> +      "devtools/client/netmonitor/src/utils/firefox/open-request-in-tab": "devtools/client/netmonitor/src/utils/open-request-in-tab",

nit: Please use `path.join()` to calcuate the relative patch like above example.

Comment 85

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

Comment on attachment 8932116 [details]
Bug 1220758 - Properly pass postData into new tab;

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

Comment 86

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

(In reply to Tim Nguyen :ntim from comment #82)
> form.method only accepts "get" and "post", so you'll probably need some code
> to handle "options"/"delete"/"patch" methods.
The new patch is using the 'form' approach only for 'post' now.

All, other comments resolved, thanks for the review Tim!

(In reply to Ricky Chien [:rickychien] from comment #84)
> nit: Please use `path.join()` to calcuate the relative patch like above
> example.
Done
Thanks!

Try is green:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0ed5cc44fb20b10e6ef9a038d00eb5598b32bc0

Honza

Comment 87

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8932116 [details]
Bug 1220758 - Properly pass postData into new tab;

https://reviewboard.mozilla.org/r/203152/#review209614

I still see some issues but I believe you can address them before landing the patch. Thanks!

::: devtools/client/netmonitor/src/utils/open-request-in-tab.js:15
(Diff revision 3)
> +/**
> + * Opens given request in a new tab.
> + */
> +function openRequestInTab(request) {
> +  let win = Services.wm.getMostRecentWindow(gDevTools.chromeWindowType);
> +  if (request.method.toLowerCase() != "get") {

nit: !==

::: devtools/client/netmonitor/src/utils/open-request-in-tab.js:34
(Diff revision 3)
> +  let form = document.createElement("form");
> +  form.target = "_blank";
> +  form.action = url;
> +  form.method = method;
> +
> +  for (let key in data) {

This could throw an error if the data in line 23 returns null.

Comment 88

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

Comment on attachment 8932116 [details]
Bug 1220758 - Properly pass postData into new tab;

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

Comment 89

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

(In reply to Ricky Chien [:rickychien] from comment #87)
Fixed.

Honza

Comment 90

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s c7beaa3f390c -d d0cbae7ed8d1: rebasing 436735:c7beaa3f390c "Bug 1220758 - Properly pass postData into new tab; r=rickychien" (tip)
merging devtools/client/netmonitor/src/request-list-context-menu.js
warning: conflicts while merging devtools/client/netmonitor/src/request-list-context-menu.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 91

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

Comment on attachment 8932116 [details]
Bug 1220758 - Properly pass postData into new tab;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/203152/diff/4-5/

Comment 92

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

Fixed, landing again.

Honza

Comment 93

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0932d85134ab
Properly pass postData into new tab; r=rickychien

Comment 94

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/0932d85134ab

Comment 95

[info]

sorry but its still not working in my FF Nightly 60... why? :-(

Comment 96

[Sebastian Zartner [:sebo]]

(In reply to info from comment #95)
> sorry but its still not working in my FF Nightly 60... why? :-(

Note that the change only affects the Network panel. The context menu option "Open URL in New Tab" within the Console panel still makes a GET request even for POST requests.

It works for me within the Network panel using 60.0a1 2018-01-22. There's still the issue I outlined in comment 7, though. With the current implementation the DevTools are not opened in the new tab, so you have to open them manually and refresh the page to see the request, which limits the functionality of this feature.

Sebastian

Comment 97

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

I can reproduce the problems. It's because post-data are fetched lazily now and they are not immediately available. If I open the request in a new tab twice it works the second time (post data are available).

This will be fixed as part of bug 1407515.

Honza

Comment 98

[Chris Mills [:cmills]]

I have had a go at documenting this, by adding a small section here:

https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor#Open_in_New_Tab

Is this OK as is, or do you think we should leave it for now, given that it isn't quite fixed?

Comment 99

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

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #98)
> I have had a go at documenting this, by adding a small section here:
> 
> https://developer.mozilla.org/en-US/docs/Tools/
> Network_Monitor#Open_in_New_Tab
Thanks!

> Is this OK as is, or do you think we should leave it for now, given that it
> isn't quite fixed?
Let's leave it there, there is good chance we'll uplift
the patch from bug 1407515.

Honza
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #98)
> I have had a go at documenting this, by adding a small section here:
> 
> https://developer.mozilla.org/en-US/docs/Tools/
> Network_Monitor#Open_in_New_Tab
> 
> Is this OK as is, or do you think we should leave it for now, given that it
> isn't quite fixed?

Comment 100

[Hani Yacoub, Desktop QA]

Since the change only affects the Network panel, I tried to verify this bug on the latest nightly and compare it to a Firefox version without the fix, but I couldn't see the differences between them.
The method POST is displayed in network tab in both versions.

Could you please clarify it a little bit for me?
Thanks.

Comment 101

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

(In reply to Hani Yacoub from comment #100)
> Since the change only affects the Network panel, I tried to verify this bug
> on the latest nightly and compare it to a Firefox version without the fix,
> but I couldn't see the differences between them.
> The method POST is displayed in network tab in both versions.
> 
> Could you please clarify it a little bit for me?
When following STR from comment #0 I am seeing
"Your HTTP request method was: GET "

... while it should say:

"Your HTTP request method was: POST "

And this is the bug.

Honza

Comment 102

[Oana Botisan, Desktop QA]

I managed to reproduce the bug using an older version of Nightly (2015-11-02) on Windows 10 x64.

I retested everything on latest Nightly 60.0a1 and beta 59.b05 using Windows 10 x64, macOS 10.13 and Ubuntu 16.04 x64 and I got the same result as in comment 97. 

Considering that this bug depends on bug 1407515, I will mark this one as verified when the other will be fixed.

Comment 103

[info]

(In reply to Sebastian Zartner [:sebo] from comment #96)
> Note that the change only affects the Network panel. The context menu option
> "Open URL in New Tab" within the Console panel still makes a GET request
> even for POST requests.
> 
> It works for me within the Network panel using 60.0a1 2018-01-22. There's
> still the issue I outlined in comment 7, though. 

So I think this was the main topic of this bug - and will this ever be fixed...?
Why cant "open in new tab" in console view just fire network panels "open in new tab" event?

[pseudo code]
console.xhr_entry.menu_actions.openinnewtab = function() {
    console.xhr_entry.getNetworkTabEntry().menu_actions.openinnewtab();
}

just an idea... cause if humans can walk this way by clicking, the code should be able, too!?

Comment 104

[Oana Botisan, Desktop QA]

I verified the fix on Ubuntu 16.04 x64, macOS 10.12, Windows 10 x64 and Windows 7 x64 using latest Nightly 66.0a1 and Dev Edition 66.0b2. The bug is not reproducing anymore.