Closed Bug 1319010 Opened 3 years ago Closed 3 years ago

Move NetworkDetailsView and SidebarView to their own modules

Categories

(DevTools :: Netmonitor, defect, P1)

defect

Tracking

(firefox53 verified)

VERIFIED FIXED
Firefox 53
Iteration:
53.1 - Nov 28
Tracking Status
firefox53 --- verified

People

(Reporter: rickychien, Assigned: rickychien)

References

(Blocks 1 open bug)

Details

(Whiteboard: [netmonitor])

Attachments

(1 file)

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.
Flags: qe-verify?
Iteration: --- → 53.1 - Nov 28
Flags: qe-verify? → qe-verify+
Priority: -- → P1
QA Contact: ciprian.georgiu
Whiteboard: [netmonitor][triage] → [netmonitor]
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.
Summary: Move NetworkDetailsView and SidebarView to its own module → Move NetworkDetailsView and SidebarView to their own modules
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
Attachment #8812720 - Flags: review?(odvarko) → review+
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.
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.
Pushed by wpan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/807095a57a80
Move NetworkDetailsView and SidebarView to their own modules r=Honza
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/807095a57a80
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Blocks: 1317649
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.