1319010 - Move NetworkDetailsView and SidebarView to their own modules

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[Ricky Chien [:rickychien] (inactive)]

After investigation, I think the SidebarView is an unnecessary component and it's easy enough to merge into NetworkDetailsView module.

This task is targeting on moving the NetworkDetailsView and SidebarView classes out of netmonitor-view.js and merging into a new NetworkDetailsView module.

Comment 1

[Ricky Chien [:rickychien] (inactive)]

I think it does make sense to keep two separate NetworkDetailsView and SidebarView modules. From component hierarchy aspect, NetworkDetailsView and CustomRequestsView are under Sidebar:

- Sidebar
  if networkDetailsOpen == true
    - NetworkDetails
  else
    - CustomRequests

So I won't merge them into NetworkDetailsView. Let's keep it as is.

Comment 2

[Ricky Chien [:rickychien] (inactive)]

Attachment: Bug 1319010 - Move NetworkDetailsView and SidebarView to their own modules

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

Comment 3

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8812720 [details]
Bug 1319010 - Move NetworkDetailsView and SidebarView to their own modules

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

Comment 4

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

Comment on attachment 8812720 [details]
Bug 1319010 - Move NetworkDetailsView and SidebarView to their own modules

https://reviewboard.mozilla.org/r/94370/#review94592

> I think it does make sense to keep two separate NetworkDetailsView and SidebarView modules.

Yes, this is better.


R+ assuming try is green.


Hope that Jarda won't have big troubles to rebase his patch.


Thanks,
Honza

Comment 5

[Ricky Chien [:rickychien] (inactive)]

browser_net_resend.js is perma failed on try and it's able to be reproducible on local. I'm trying to find out the root cause now.

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8812720 [details]
Bug 1319010 - Move NetworkDetailsView and SidebarView to their own modules

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

Comment 7

[Ricky Chien [:rickychien] (inactive)]

Timeout in browser_net_resend.js is due to promise compatibility. I replaced promise.jsm with standard Promise accidentally that caused the test timeout.

I will not change all promise.jsm to standard Promise since that will affect the work of bug 1309866.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/807095a57a80
Move NetworkDetailsView and SidebarView to their own modules r=Honza

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/807095a57a80

Comment 10

[Ciprian Georgiu, Desktop QA]

This issue is verified fixed on latest Nightly 53.0a1 (2016-11-23) on the following OSes:
- Windows 10 x64
- Ubuntu 16.04 x64
- Mac OS X 10.11.6
I didn't encountered any issues on this fix, based on this I will mark here accordingly.