Closed
Bug 1403927
Opened 7 years ago
Closed 7 years ago
HTTP inspector do not show parameters of a POST request
Categories
(DevTools :: Console, defect, P1)
DevTools
Console
Tracking
(Not tracked)
VERIFIED
FIXED
Firefox 58
People
(Reporter: nchevobbe, Assigned: Honza)
References
Details
(Whiteboard: [reserve-console-html])
Attachments
(4 files)
Steps to reproduce:
1. go to https://polar-wood.glitch.me/
2. open the netmonitor
3. open the split console, and activate "xhr" filter
4. Enter something in the input and submit it
5. In the netmonitor, click on the newly shown entry, and click on the parameters tab
6. In the console expand on the newly shown message, and click on the parameters tab
Expected results:
The HTTP inspector should show the same thing that in the netmonitor
Actual results:
The HTTP inspector has an empty parameters panel (see attachment), and the following error is logged to the browser console:
"TypeError: undefined is not a valid URL. request-utils.js:146:11"
Reporter | ||
Updated•7 years ago
|
Has STR: --- → yes
status-firefox57:
--- → fix-optional
tracking-firefox57:
--- → ?
Priority: -- → P2
Whiteboard: [console-html][triage]
Updated•7 years ago
|
Flags: qe-verify+
Priority: P2 → P3
QA Contact: iulia.cristescu
Whiteboard: [console-html][triage] → [reserve-console-html]
Assignee | ||
Comment 1•7 years ago
|
||
I believe that this will be fixed by bug 1404138.
In any case, this report depends on bug 1404138 and should be retested as soon it's fixed.
Honza
Depends on: 1404138
Comment 3•7 years ago
|
||
Remains broken after bug 1404138 was resolved
Reporter | ||
Comment 4•7 years ago
|
||
Andrew, where are you seeing the issue ? i built from mozilla-central and it worked for me.
If you are on nightly, the fix will be in there tomorrow i think
Flags: needinfo?(andrewmoffett67)
Comment 5•7 years ago
|
||
Apologies I assumed this would have been merged into todays nightly, I'll test again tomorrow.
Flags: needinfo?(andrewmoffett67)
Comment 6•7 years ago
|
||
Was this merged? Its still broken for me in 58.0a1 (2017-10-05) (64-bit)
Comment 7•7 years ago
|
||
Assignee | ||
Comment 8•7 years ago
|
||
Icant (In reply to Andrew Moff from comment #6)
> Was this merged?
Yes
I've been testing this on latest Nightly + Win10 and it works for me.
Anyone is able to reproduce this issue?
Honza
Reporter | ||
Comment 9•7 years ago
|
||
works for me with the steps to reproduce from Comment 0, on 58.0a1 (2017-10-05) (64-bit) (see attachment)
Andrew, do you see the issue if you follow the steps from comment 0 ? If you do, there must be something weird. And if you don't see the issue for this case, can you provide more information about your application ?
Flags: needinfo?(andrewmoffett67)
Comment 10•7 years ago
|
||
The test outlined by the OP isn't a good test its simply appending the url with a query string, its not actually posting any params in the body.
Try this test here. http://janodvarko.cz/firebug/tests/601/Issue601.htm
https://imgur.com/a/qz12w
Flags: needinfo?(andrewmoffett67)
Assignee | ||
Comment 11•7 years ago
|
||
(In reply to Andrew Moff from comment #10)
> The test outlined by the OP isn't a good test its simply appending the url
> with a query string, its not actually posting any params in the body.
>
> Try this test here. http://janodvarko.cz/firebug/tests/601/Issue601.htm
Yes, I can reproduce the problem with my test :-)
Thanks!
Honza
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•7 years ago
|
||
I did analyze the problem and attached a prototype patch.
Some comments:
1) The main problem is that the code responsible for parsing POSTed data is part of MonitorPanel component (Net panel). This component is parent of all the detail panels in the Net panel, but not used in HTTPi.
2) The patch moves POST data parsing logic from MonitorPanel to ParamsPanel, so it's also executed by HTTPi. This is better location for that logic anyway.
3) The patch makes also sure that Console reducer is handling `UPDATE_REQUEST` action.
4) There is one hack related to 'getLongString' API used by the ParamsPanel now. There is currently now way to pass custom `getLongString` method into HTTPi from WebConsole. This is already solved by bug 1005755, so I am marking it as a blocker for this report.
Honza
Depends on: 1005755
Updated•7 years ago
|
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Priority: P3 → P1
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•7 years ago
|
||
Just note for me.
Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e9da10c1eca75825bc2aeda8b2a2a68684d9eff&selectedJob=137518877
Failures:
FAIL | devtools/client/netmonitor/test/browser_net_complex-params.js | Test timed out
FAIL | devtools/client/netmonitor/test/browser_net_post-data-01.js | Test timed out
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 20•7 years ago
|
||
Test failures from comment #17 fixed.
New try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5097d134b90881df5d69ec865377f3061790fa8c
Honza
Reporter | ||
Comment 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8916521 [details]
Bug 1403927 - Fix Params panel in HTTPi;
https://reviewboard.mozilla.org/r/187654/#review195472
thanks honza. I'm not sure about the connection part on the ParamsPanel, could you answer my comment ?
::: devtools/client/netmonitor/src/components/params-panel.js:38
(Diff revision 3)
> -/*
> +/**
> * Params panel component
> * Displays the GET parameters and POST data of a request
> */
> -function ParamsPanel({
> +const ParamsPanel = createClass({
This sound like it can make things a bit slower since we turn this into a stateful component (and monitor-panel.js is still a staetful one too)
There might be no solution to this, just wanted to point that out.
::: devtools/client/netmonitor/src/components/params-panel.js:183
(Diff revision 3)
> -module.exports = ParamsPanel;
> +module.exports = connect(
> + (state) => ({
> + }),
> + (dispatch) => ({
> + updateRequest: (id, data, batch) => dispatch(Actions.updateRequest(id, data, batch)),
> + }),
> +)(ParamsPanel);
do we really want to connect the component ? it only seems to be fore getting the dispatch function.
If we don't want to render on redux changes here (it might already be the case because of parent components), we may only pass dispatch as a prop ? (or updateRequest)
Reporter | ||
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8919271 [details]
Bug 1403927 - Test for HTTPi Params panel;
https://reviewboard.mozilla.org/r/190164/#review195760
The test looks good to me. I have a minor comment, but I'm fine if it lands as is. Thanks !
::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_network_messages_expand.js:105
(Diff revision 2)
> resolve();
> });
> });
> }
>
> -async function waitForSourceEditor(messageNode) {
> +async function waitForSourceEditor(messageNode, selector) {
maybe we could pass it the tab (paramsTab, responseTab), and query on it ?
```
function waitForSourceEditor(tabNode) {
return waitFor(() => tabNode.querySelector(".CodeMirror"));
}
```
I find it easier than to deal with selectors. Unless we can have multiple CodeMirror instances in one panel ?
Also, I think we can remove `async` since we don't use await here.
Attachment #8919271 -
Flags: review?(nchevobbe) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 26•7 years ago
|
||
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #21)
> > -function ParamsPanel({
> > +const ParamsPanel = createClass({
>
> This sound like it can make things a bit slower since we turn this into a
> stateful component (and monitor-panel.js is still a staetful one too)
> There might be no solution to this, just wanted to point that out.
I need 'componentDidMount' and 'componentWillReceiveProps',
so I can't use a function, I guess.
> > +module.exports = connect(
> > + (state) => ({
> > + }),
> > + (dispatch) => ({
> > + updateRequest: (id, data, batch) => dispatch(Actions.updateRequest(id, data, batch)),
> > + }),
> > +)(ParamsPanel);
> do we really want to connect the component ? it only seems to be fore
> getting the dispatch function.
> If we don't want to render on redux changes here (it might already be the
> case because of parent components), we may only pass dispatch as a prop ?
> (or updateRequest)
Yeah, I kept it there eventually since there is only one instance of that
panel and passing explicit dispatch prop is weird. But, let me know in case.
Try push green!
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b24a37fdbe413ada3aa72e759c690620bd63befd
I still hope we can remove the NUMBER_OF_NETWORK_UPDATE thing,
but in another bug I guess.
Honza
Reporter | ||
Comment 27•7 years ago
|
||
mozreview-review |
Comment on attachment 8916521 [details]
Bug 1403927 - Fix Params panel in HTTPi;
https://reviewboard.mozilla.org/r/187654/#review196190
Attachment #8916521 -
Flags: review?(nchevobbe) → review+
Comment 28•7 years ago
|
||
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6b88d856e00f
Fix Params panel in HTTPi; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/ac5ebd3c7e82
Test for HTTPi Params panel; r=nchevobbe
Comment 29•7 years ago
|
||
Backed out for failing mochitest-clipboard's devtools/client/netmonitor/test/browser_net_copy_params.js:
https://hg.mozilla.org/integration/autoland/rev/e8acb0a73375d4b08fa0152c03f15b640c81fa52
https://hg.mozilla.org/integration/autoland/rev/879e6055ac0927eca638945c371d0959b594fab0
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ac5ebd3c7e8299b479167760d0ec0b277c6b3065&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=138140891&repo=autoland
[task 2017-10-19T09:09:07.585Z] 09:09:07 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_copy_params.js | The "Copy URL Parameters" context menu item should not be hidden. -
[task 2017-10-19T09:09:07.586Z] 09:09:07 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_copy_params.js | Clipboard has the given value -
[task 2017-10-19T09:09:07.586Z] 09:09:07 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_copy_params.js | The url query string copied from the selected item is correct. -
[task 2017-10-19T09:09:07.586Z] 09:09:07 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_copy_params.js | The "Copy POST Data" context menu item should not be hidden. -
[task 2017-10-19T09:09:07.587Z] 09:09:07 INFO - Buffered messages finished
[task 2017-10-19T09:09:07.587Z] 09:09:07 INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_copy_params.js | Test timed out -
Flags: needinfo?(odvarko)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 32•7 years ago
|
||
Interesting is that I didn't see that failure in my try push:
Patch updated, fixing the failure.
Nicolas: the logic that parses urlencoded POST data needs to happen at both places:
1) MonitorPanel: when the user click on a request in the Net panel, the context menu offers "Copy POST DATA". The side bar (MonitorPanel) needs to parse the data at that moment, so the context menu works.
2) ParamsPanel: MonitorPanel is not used in the Console panel, so ParamsPanel needs to care.
Honza
Flags: needinfo?(odvarko)
Comment 33•7 years ago
|
||
(In reply to Jan Honza Odvarko [:Honza] from comment #32)
> Interesting is that I didn't see that failure in my try push
I think that's because it was in the mochitest-clipboard job. If you change your try push syntax from `-u mochitest-dt` to `-u mochitests` it will catch issues like that.
Comment 34•7 years ago
|
||
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b263ad73a219
Fix Params panel in HTTPi; r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/37da7b7c6a40
Test for HTTPi Params panel; r=nchevobbe
Comment 35•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b263ad73a219
https://hg.mozilla.org/mozilla-central/rev/37da7b7c6a40
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment 37•7 years ago
|
||
I'm trying to verify this issue on the latest nightly, but I couldn't reproduce it because while trying to access the first website from comment 0 (https://polar-wood.glitch.me/) a message was displayed that the Project Not Found.
Could you please provide another website so I could verify this bug?
Thanks.
Flags: needinfo?(nchevobbe)
Reporter | ||
Comment 38•7 years ago
|
||
Sorry Hani, you can go to https://console-html-network-requests.glitch.me/ , click the button and expand the GET & POST requests and check that there are indeed params shown there.
Flags: needinfo?(nchevobbe)
Comment 39•7 years ago
|
||
Verified as fixed on Firefox Nightly 59.0a1 and Firefox 58.0b3 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.
Comment 40•7 years ago
|
||
Ubuntu 16.04 just upgraded my Firefox to 57.0 and now I can't see any POST params in the inspector. Is this actually fixed now? Should I chase Ubuntu to release a patch? Is there a workaround - apart from Use Chrome ;-) ?
Comment 43•7 years ago
|
||
Still not resolved in nightly 19.10.2017. See 1410705. If the request takes long enough to allow you to open the params tab before the server has responded, no headers, cookies, params, response, timings are shown.
Comment 44•7 years ago
|
||
(In reply to gvl from comment #43)
> Still not resolved in nightly 19.10.2017. See 1410705. If the request takes
> long enough to allow you to open the params tab before the server has
> responded, no headers, cookies, params, response, timings are shown.
I'm reposting that comment here since 1410705 was marked as a duplicate:
I've been testing it out, and here's what I've found:
The console is now showing information correctly on all tabs (Headers, Cookies, Params, Response, and Timings) EXCEPT when I try to open any of this information before the request is complete. If I attempt to view any tab before the request has resolved, then none of it will populate after it has resolved. If I leave it alone until it resolves, then all data is populated properly.
I'm running Windows 10 and Firefox 58.0b4 (64-bit) (Dev edition).
Assignee | ||
Comment 45•7 years ago
|
||
(In reply to meir_anolick from comment #44)
> (In reply to gvl from comment #43)
> > Still not resolved in nightly 19.10.2017. See 1410705. If the request takes
> > long enough to allow you to open the params tab before the server has
> > responded, no headers, cookies, params, response, timings are shown.
>
> I'm reposting that comment here since 1410705 was marked as a duplicate:
>
> I've been testing it out, and here's what I've found:
> The console is now showing information correctly on all tabs (Headers,
> Cookies, Params, Response, and Timings) EXCEPT when I try to open any of
> this information before the request is complete. If I attempt to view any
> tab before the request has resolved, then none of it will populate after it
> has resolved. If I leave it alone until it resolves, then all data is
> populated properly.
>
> I'm running Windows 10 and Firefox 58.0b4 (64-bit) (Dev edition).
Please create a new report for it, this bug is already closed.
Also, having a test case (can be an online page) helps a lot.
You can cc my on that new bug.
Thanks!
Honza
Comment 49•7 years ago
|
||
Is it possible to backport it to Firefox 57? It is kind of a bug and regression.
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•6 years ago
|
status-firefox57:
wontfix → ---
status-firefox58:
verified → ---
status-firefox59:
verified → ---
tracking-firefox57:
- → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•