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

VERIFIED FIXED in Firefox 59

Status

defect
P2
normal
VERIFIED FIXED
4 years ago
3 months ago

People

(Reporter: donv, Assigned: Honza, Mentored)

Tracking

(Blocks 2 bugs, {dev-doc-complete, DevAdvocacy, good-first-bug})

41 Branch
Firefox 59
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed, firefox66 verified)

Details

(Whiteboard: [DevRel:P3][netmonitor-reserve])

Attachments

(1 attachment, 6 obsolete attachments)

(Reporter)

Description

4 years ago
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.
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.
Status: UNCONFIRMED → NEW
Component: Untriaged → Developer Tools: Netmonitor
Ever confirmed: true
Keywords: DevAdvocacy
Whiteboard: [DevRel:P3]
(Reporter)

Comment 2

3 years ago
(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

2 years ago
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='
Duplicate of this bug: 1160610
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
Blocks: firebug-gaps
Keywords: dev-doc-needed
See Also: → 1257833, 1031956
Summary: Network Tab "Open in New Tab" Feature Won't POST → "Open in New Tab" on POST requests makes a GET request

Comment 6

2 years ago
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.
Priority: -- → P2
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
Priority: P2 → --
Priority: -- → P2

Comment 8

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

Comment 9

2 years ago
(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.

Updated

2 years ago
Duplicate of this bug: 1337685

Comment 11

2 years ago
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.
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
Mentor: odvarko
Has STR: --- → yes
Whiteboard: [DevRel:P3] → [DevRel:P3][netmonitor-reserve]

Comment 13

2 years ago
Hi, may I work on this bug? This will be my GFB for school assignment.
(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
Assignee: nobody → guohao.yan
Status: NEW → ASSIGNED
(Reporter)

Comment 15

2 years ago
Thank you :)

This feature is very important to me!
Flags: qe-verify?
Priority: P2 → P3
Flags: qe-verify? → qe-verify+
Hi Don, 
any progress on this?
Do you need help?

Honza
QA Contact: ciprian.georgiu
(Reporter)

Comment 17

2 years ago
The bug is still present in Firefox 51.0.1 on Xenial :(
(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
(Reporter)

Comment 19

2 years ago
(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

2 years ago
(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

2 years ago
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 :(
> 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.
Flags: needinfo?(guohao.yan)

Comment 23

2 years ago
(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?
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

2 years ago
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);
     }
  },
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?
Flags: needinfo?(odvarko)

Comment 27

2 years ago
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

2 years ago
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.
(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

Updated

2 years ago
Duplicate of this bug: 1337295
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).
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?
Flags: needinfo?(odvarko)
Summary: "Open in New Tab" on POST requests makes a GET request → Rephase `Open in a new tab`to `Resend this request`
(Reporter)

Comment 33

2 years ago
(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?
Flags: needinfo?(gasolin)
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?
Flags: needinfo?(gasolin) → needinfo?(donv)
Blocks: netmonitor-phaseII
No longer blocks: netmonitor-html

Comment 35

2 years ago
(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!

;-)
(Reporter)

Comment 36

2 years ago
(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" :)
Flags: needinfo?(donv)
(Reporter)

Comment 37

2 years ago
(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

2 years ago
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?
Flags: needinfo?(guohao.yan) → needinfo?(gasolin)
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?
Flags: needinfo?(gasolin) → needinfo?(odvarko)
Summary: Rephase `Open in a new tab`to `Resend this request` → "Open in New Tab" on POST requests makes a GET request
(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
Flags: needinfo?(odvarko)

Comment 41

2 years ago
(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"}
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

2 years ago
Posted patch bug1220758 (obsolete) — Splinter Review
Hi, Thanks for all the help, I've created the patch file in attachments.

Updated

2 years ago
Attachment #8854514 - Flags: review?(odvarko)

Comment 44

2 years ago
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 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).
Attachment #8854514 - Flags: review?(odvarko) → review-
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)
(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
Flags: needinfo?(rchien)

Comment 48

2 years ago
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

2 years ago
Posted patch adding revised patch file (obsolete) — Splinter Review
Attachment #8855296 - Flags: review?(odvarko)

Comment 50

2 years ago
sorry.....something went wrong please ignore previous patch file...

Comment 51

2 years ago
Posted patch bug1220758.patch (obsolete) — Splinter Review
Attachment #8855313 - Flags: review?(odvarko)
(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
Attachment #8855296 - Attachment is obsolete: true
Attachment #8855296 - Flags: review?(odvarko)
Attachment #8854514 - Attachment is obsolete: true
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
Attachment #8855313 - Flags: review?(odvarko) → review-
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.
(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

2 years ago
Not sure about the decision of moving this into utli/chrome/ is it confirmed and I should be doing it?
Attachment #8855564 - Flags: review?(odvarko)

Comment 57

2 years ago
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 ..
Attachment #8855564 - Attachment is obsolete: true
Attachment #8855564 - Flags: review?(odvarko)

Comment 58

2 years ago
Posted patch bug1220758.patch (obsolete) — Splinter Review
Attachment #8855570 - Flags: review?(odvarko)
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.
Flags: needinfo?(guohao.yan)
(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.
Flags: needinfo?(rchien)
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.
As offline discussion, we agree to put non-Web-APIs(for this case, open-request-in-tab.js) into `netmonitor/src/utils/chrome/`.
(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

2 years ago
Posted patch bug1220758.patch (obsolete) — Splinter Review
Combined the 2 patch files.
Attachment #8855313 - Attachment is obsolete: true
Attachment #8855570 - Attachment is obsolete: true
Attachment #8855570 - Flags: review?(odvarko)
Flags: needinfo?(guohao.yan)
Attachment #8856139 - Flags: review?(odvarko)

Comment 65

2 years ago
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 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
Attachment #8856139 - Flags: review?(odvarko) → review-
Guohao, are you still working on this patch?

Sebastian
Flags: needinfo?(guohao.yan)

Comment 68

2 years ago
+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 hidden (me-too)
Comment hidden (me-too)
Comment hidden (me-too)
Priority: P3 → P2
Comment hidden (me-too)
@guohao: are you planning to finish this?
Do you need any help or should I do it?

Honza

Comment 74

a year ago
@guohao / @Honza, are you working on it?
Thanks
Comment hidden (mozreview-request)
Assignee: guohao.yan → odvarko
Attachment #8856139 - Attachment is obsolete: true

Comment 76

a year ago
mozreview-review
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 ?
(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

a year ago
mozreview-review
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.
Attachment #8932116 - Flags: review?(rchien) → review-
(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 hidden (mozreview-request)
(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
Flags: needinfo?(ntim.bugs)

Comment 82

a year ago
mozreview-review
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.
Added some comments
Flags: needinfo?(ntim.bugs)

Comment 84

a year ago
mozreview-review
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.
Attachment #8932116 - Flags: review?(rchien) → review-
Comment hidden (mozreview-request)
(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

a year ago
mozreview-review
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.
Attachment #8932116 - Flags: review?(rchien) → review+
Comment hidden (mozreview-request)
(In reply to Ricky Chien [:rickychien] from comment #87)
Fixed.

Honza
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 hidden (mozreview-request)
Fixed, landing again.

Honza

Comment 93

a year ago
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0932d85134ab
Properly pass postData into new tab; r=rickychien

Comment 94

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0932d85134ab
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Flags: needinfo?(guohao.yan)

Comment 95

a year ago
sorry but its still not working in my FF Nightly 60... why? :-(
(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
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
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?
Flags: needinfo?(odvarko)
(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?
Flags: needinfo?(odvarko)

Comment 100

a year ago
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.
Depends on: 1407515
Flags: needinfo?(odvarko)
(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
Flags: needinfo?(odvarko)
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

a year ago
(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!?

Updated

11 months ago
Product: Firefox → DevTools

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.