Closed Bug 1479421 Opened Last year Closed Last year

Response payload not visible

Categories

(DevTools :: Netmonitor, defect, P3)

62 Branch
x86_64
Linux
defect

Tracking

(firefox64 fixed)

RESOLVED FIXED
Firefox 64
Tracking Status
firefox64 --- fixed

People

(Reporter: mail, Assigned: amychan331, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: good-first-bug, )

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.75 Safari/537.36

Steps to reproduce:

Load a large JSON file and look at it in the network panel, in the response tab. Make sure both "JSON" and "Response payload" are open.



Actual results:

When the JSON is long enough (more content than the height of the devtools window), the response payload is not visible, even though it is open. See attached screenshot in the bottom ("Charge utile de la réponse").


Expected results:

The response payload should have been visible
Component: Untriaged → Netmonitor
OS: Unspecified → Linux
Product: Firefox → DevTools
Hardware: Unspecified → x86_64
Thanks for the report I can reproduce the problem on my Win10 machine.

Honza
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Some info for anyone interested in fixing this bug:

1) Both response sections are rendered/generated here:
https://searchfox.org/mozilla-central/rev/f0c15db995198a1013e1c5f5b5bea54ef83f1049/devtools/client/netmonitor/src/components/ResponsePanel.js#190-203

The code belongs to ResponsePanel, which is React component

2) The problem is that the "Response payload" section body height is 0 when the JSON section is opened. It's likely because the first section gets all the available vertical space and the "Response payload" section body doesn't have min-height set...

Use Browser Toolbox Inspector to inspect the Network monitor UI (specifically the Response side panel) to see it.
https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox

Honza
Mentor: odvarko
Keywords: good-first-bug
Whiteboard: good-first-bug,
Hi! Can I work on this one?
Making the response panel visible
Got a bit stuck on trying to change just the height - turns out it was the overflow property that was causing the problem! Also got stuck on the change from hg to Phabricator, but I think I did it right? Please let me know if I made any mistake in the phabricator settings...
Flags: needinfo?(odvarko)
(In reply to Amy Chan from comment #5)
> Got a bit stuck on trying to change just the height - turns out it was the
> overflow property that was causing the problem! Also got stuck on the change
> from hg to Phabricator, but I think I did it right? Please let me know if I
> made any mistake in the phabricator settings...

The patch is properly uploaded in Phabricator and I just commented on it.

Thanks for the help!
Honza
Flags: needinfo?(odvarko)
Comment on attachment 9005099 [details]
Bug 1479421 - Making the response panel visible; r?Honza

Jan Honza Odvarko [:Honza] has approved the revision.
Attachment #9005099 - Flags: review+
One more thing, can we also have a test for this? (to avoid regression).

Here is an example test you can get some inspiration from:
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/test/browser_net_json-long.js
(we coulc just extend this one I think)

You can also search for `#response-panel` in the devtools/client/netmonitor/test directory to see other related tests.

It's ok for me if you want to do this as part of another bug report.

Honza
Assignee: nobody → amy_yyc
Status: NEW → ASSIGNED
Flags: needinfo?(amy_yyc)
Pushed a new update with the comment and testing - so I just add to the browser_net_json-long.js? Is it just checking for the editor container existence like this?
Flags: needinfo?(amy_yyc)
Comment on attachment 9005099 [details]
Bug 1479421 - Making the response panel visible; r?Honza

Jan Honza Odvarko [:Honza] has requested changes to the revision.
Attachment #9005099 - Flags: review+
Comment on attachment 9005099 [details]
Bug 1479421 - Making the response panel visible; r?Honza

Jan Honza Odvarko [:Honza] has approved the revision.
Attachment #9005099 - Flags: review+
@Amy: The patch works for me, thanks! Don't forget to land it.

Honza
Flags: needinfo?(amy_yyc)
Unfortunately, I don't have access to make the land - it seems I don't have LDAP account to log into Lando? Is it ok if you help me land it?
Flags: needinfo?(amy_yyc) → needinfo?(odvarko)
Done

Thanks!
Honza
Flags: needinfo?(odvarko)
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f8a7ea258709
Making the response panel visible; r=Honza
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/netmonitor/test/browser_net_json-long.js:105:1 | Block must not be padded by blank lines. (padded-blocks)

@Amy: this is simple to fix, just remove one unnecessary empty line.
Can you please take a quick look, so we can land it again?

Thanks!
Honza
Just pushed the latest changes!
Flags: needinfo?(amy_yyc)
I thought this would have to be landed again because of push failure from ES lint, but I just check Phabricator & realized it was landed - I guessed since it was landed once, it remained landed? Should I changed the status of this bug to resolved?
Flags: needinfo?(odvarko)
(In reply to Amy Chan from comment #19)
> I thought this would have to be landed again because of push failure from ES
> lint, but I just check Phabricator & realized it was landed - I guessed
> since it was landed once, it remained landed?
It isn't landed.

You need to remove that line and update the patch in phabricator (you might need to reopen that in Phabricator).

> Should I changed the status of this bug to resolved?
No, it'll happen automatically as soon as it's properly landed.

Honza
Flags: needinfo?(odvarko) → needinfo?(amy_yyc)
Comment on attachment 9005099 [details]
Bug 1479421 - Making the response panel visible; r?Honza

Jan Honza Odvarko [:Honza] has requested changes to the revision.
Attachment #9005099 - Flags: review+
Actually, I already removed the line and updated the patch earlier - it's the empty line in 104 for Diff 14070, right? Is there another line that I need to remove?
I also tried to reopen the revision in Phabricator following the instruction here: https://wiki.mozilla.org/Phabricator/FAQ#How_do_I_reopen_an_existing_revision_to_submit_more_updates_for_review_.28e.g._following_a_backout.29.3F
But my Revision Actions tab in the dropdown don't have the Reopen Revision option as indicated. Is there another way to reopen it?
Flags: needinfo?(amy_yyc) → needinfo?(odvarko)
Comment on attachment 9005099 [details]
Bug 1479421 - Making the response panel visible; r?Honza

Jan Honza Odvarko [:Honza] has approved the revision.
Attachment #9005099 - Flags: review+
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/255ba5eac283
Making the response panel visible; r=Honza
Trying to re-land now...
Honza
Flags: needinfo?(odvarko)
https://hg.mozilla.org/mozilla-central/rev/255ba5eac283
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Depends on: 1498565
You need to log in before you can comment on or make changes to this bug.