Closed Bug 1315175 Opened 5 years ago Closed 5 years ago

Move Netmonitor context menu code to a separate module

Categories

(DevTools :: Netmonitor, defect, P1)

defect

Tracking

(firefox52 fixed)

RESOLVED FIXED
Firefox 52
Iteration:
52.3 - Nov 14
Tracking Status
firefox52 --- fixed

People

(Reporter: jsnajdr, Assigned: jsnajdr)

References

(Blocks 1 open bug)

Details

(Whiteboard: [netmonitor])

Attachments

(1 file)

The context menu code in requests-menu-view.js should be moved to a separate module (request-list-context-menu.js?).

This involves the _openMenu method and all the handlers (copyUrl etc.)

This will make the requests-menu-view.js file simpler and the changes in bug 1309866 easier to understand and review.
Whiteboard: [netmonitor][triage]
Whiteboard: [netmonitor][triage] → [netmonitor]
Priority: -- → P2
Moved the context menu code to a separate "RequestListContextMenu" object. This removes almost 20% of code away from requests-menu-view.js, which is very nice.

The new module contains the code for showing the menu, plus all the command handlers. The tests needed only simple updates.

I'd ask Fred for the review, but he'll be on WebSummit for the whole following week.
Assignee: nobody → jsnajdr
Status: NEW → ASSIGNED
Iteration: --- → 52.3 - Nov 14
Priority: P2 → P1
Comment on attachment 8807591 [details]
Bug 1315175 - Move Netmonitor context menu code to a separate module

https://reviewboard.mozilla.org/r/90714/#review90398

Nice!

I tested this and all work fine for me.

Just a few nits inline.

R+ assuming that Try is green and my comments resolved.

Thanks Jarda!

Honza

::: devtools/client/netmonitor/request-list-context-menu.js:6
(Diff revision 1)
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/* globals NetMonitorController, NetMonitorView, gNetwork */
> +"use strict";

nit: missing empty line above "use strict"

::: devtools/client/netmonitor/requests-menu-view.js:1
(Diff revision 1)
>  /* globals document, window, dumpn, $, gNetwork, EVENTS, Prefs,

Missing Mozilla license header

::: devtools/client/netmonitor/requests-menu-view.js:3
(Diff revision 1)
>  /* globals document, window, dumpn, $, gNetwork, EVENTS, Prefs,
>             NetMonitorController, NetMonitorView */
>  "use strict";

nit: missing empty line above and below "use strict"
Attachment #8807591 - Flags: review?(odvarko) → review+
(In reply to Jarda Snajdr [:jsnajdr] from comment #2)
> Moved the context menu code to a separate "RequestListContextMenu" object.
> This removes almost 20% of code away from requests-menu-view.js, which is
> very nice.
Yes!

Honza
Fixed issues from review. Try run has some timeout failures on Netmonitor, but that's caused by bug 1315635, not related to this one. Going to land.
Pushed by jsnajdr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/018f16f5f08e
Move Netmonitor context menu code to a separate module r=Honza
https://hg.mozilla.org/mozilla-central/rev/018f16f5f08e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.