1315175 - Move Netmonitor context menu code to a separate module

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect, P1)

Description

[Jarda Snajdr [:jsnajdr]]

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.

Comment 1

[Jarda Snajdr [:jsnajdr]]

Attachment: Bug 1315175 - Move Netmonitor context menu code to a separate module

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

Comment 2

[Jarda Snajdr [:jsnajdr]]

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.

Comment 3

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

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"

Comment 4

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

(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

Comment 5

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8807591 [details]
Bug 1315175 - Move Netmonitor context menu code to a separate module

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

Comment 6

[Jarda Snajdr [:jsnajdr]]

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.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/018f16f5f08e
Move Netmonitor context menu code to a separate module r=Honza

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/018f16f5f08e