1588478 - devtools.netmonitor.panes-network-details-height keeps on resetting to 50

Status: NEW

(DevTools :: Netmonitor, defect, P3)

Description

[SL]

User Agent: Mozilla/5.0 (Windows NT 6.3; rv:69.0) Gecko/20100101 Firefox/69.0

Steps to reproduce:

1. Open the developer tools (make sure it's docked to the left/right and its width less than 700px*) and go to an example site.
2. Switch to the network monitor and expand the details pane (by clicking on any request).
3. Switch to another tool (e.g. Console).
4. Close the developer tools.

*The width of the developer tools needs to be less than 700px for the details pane to appear at the bottom, not at the right.

Actual results:

If the user opens the developer tools and navigates to the network monitor again, expanding the details pane once more shows that the height of the details pane resets to 50px (this is reflected in both about:config with the configuration named "devtools.netmonitor.panes-network-details-height" as well as visually - the height is reduced significantly).

Expected results:

The height of the details pane of the network monitor should always remain unchanged.

Comment 1

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

Thanks for the report!
I can see the issue on my machine (Win10, Nightly)
Honza

Comment 2

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

Some instructions for anyone interested in fixing this:

1. Size of the Details pane is stored here:
   https://searchfox.org/mozilla-central/rev/6866d3a650c826f1cabd123663e84b95ee743701/devtools/client/netmonitor/src/components/MonitorPanel.js#110

2. The height should be stored in "devtools.netmonitor.panes-network-details-height" pref

3. We should use the Browser Toolbox
   https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox
   (or console.log statements)

...and verify that the method in #1 is properly called and storing/reading the expected value.

Honza

Comment 3

[Pranav pandey]

Hi, 👋
I did some digging so the problem is that whenever we move over to any other tab except for(network tab) the height of NetworkActionBar becomes 0 as expected but when we close this(https://searchfox.org/mozilla-central/rev/6866d3a650c826f1cabd123663e84b95ee743701/devtools/client/netmonitor/src/components/MonitorPanel.js#111) return 0, then after you open it again it will take 50 default, we can use networkDetailsHeight from UI reducer instead of it . What do you guys think..

Pranav

Comment 4

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

Thanks Pranav for looking into this!

Yes, it make sense to read the value from theUI reduce.

Note that the networkDetailsHeight value (and networkDetailsWidth) is updated only when RESIZE_NETWORK_DETAILS action is fired and we might want to make sure that value is always correct when we read it. I.e. we might want to initialize the value at the beginning to make it valid even if the user is not resizing the details panel.

If you have a patch I am happy to test it.

Honza

Comment 5

[Pranav pandey]

Attachment: Bug 1588478 - networkDetailPanel size gets saved form ui reducer and not from ref.

Comment 6

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

Thanks for the patch, some comments left in Phabricator.

Honza

Comment 7

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

Attachment: Use "pref" middleware

@Pranav: please look at this patch that indicates the path forward

We should do similar thing for `persistActionBarSize` too

Honza

Comment 8

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

Attachment: temp.patch

@Pranav: please look at this patch showing how we can fix persistence for the Actions bar as well.

Honza

Comment 9

[Phabricator Automation]

Comment on attachment 9122855 [details]
Bug 1588478 - networkDetailPanel size gets saved form ui reducer and not from ref.

Revision D60951 was moved to bug 1588477. Setting attachment 9122855 [details] to obsolete.

Comment 10

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

Pranav, you are assigned to this bug, but there is no activity for a long time. Are you still planning to work on this?
(it looks like the patch is close, there is just one failing test)

Honza

Comment 11

[Pranav pandey]

Sorry , Honza for the delay, problem I am having the problem with test it worked for me on my mac but not for you on your windows, I don't have a windows PC and due lockdown I was not able to arrange one powerful enough ,I was thinking that after the lockdown opens I could look that , maybe you could look at it I am rebasing the patch and updating it , It might fix it , the problem is that the new Test that I wrote modify the width/height of the network panel by dragging it
the previous test was just setting the width/height of the panel to x value and then testing if it got saved getDoc().querySelector(".monitor-panel .split-box .controlled").style.height = `${value}px`;) we cannot do that as we changed this behavior

the problem with dragging is getting value exactly write for test is kind of hard , maybe I am doing some mistake,

Pranav

Comment 12

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

(In reply to Pranav pandey from comment #11)

I don't have a windows PC
Could you use VirtualBox or VMware?

The patch looks good so, it's only the test

Honza

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.