1380927 - Fix context menu alias

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement)

Description

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

This is a follow up for bug 1376291.

Webpack alias feature doesn't support relative URLs. So, we need to change "./utils/menu" URL to something like "devtools/client/netmonitor/src/utils/menu" and then properly alias it in Net monitor's webpack config (used when loading Net panel on top of the Launchpad).

Honza

Comment 1

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

Attachment: Bug 1380927 - Fix context menu alias;

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

Comment 2

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

(In reply to Ricky Chien [:rickychien] from comment #11)
> Webpack alias cannot support ./xxx/xxx pattern [1]. So the only way you can
> do for supporting both toolbox and launchpad is to change "./utils/menu" at
> to "devtools/client/netmonitor/src/utils/menu" and then update the
> webpack alias in webpack.config.js to
> "devtools/client/netmonitor/src/utils/menu"

Yes, that's exactly what I've been doing to fix the problem, but I am still seeing:

ERROR in C:/src/mozilla.org/mozilla-central/devtools/client/netmonitor/src/request-list-header-context-menu.js
Module not found: Error: Cannot resolve module 'devtools-launchpad/src/components/shared/menu' in C:\src\mozilla.org\mozilla-central\devtools\client\netmonitor\src
 @ C:/src/mozilla.org/mozilla-central/devtools/client/netmonitor/src/request-list-header-context-menu.js 9:21-73

ERROR in C:/src/mozilla.org/mozilla-central/devtools/client/netmonitor/src/request-list-context-menu.js
Module not found: Error: Cannot resolve module 'devtools-launchpad/src/components/shared/menu' in C:\src\mozilla.org\mozilla-central\devtools\client\netmonitor\src
 @ C:/src/mozilla.org/mozilla-central/devtools/client/netmonitor/src/request-list-context-menu.js 22:21-73


Any tips?

Honza

Comment 3

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

Comment on attachment 8886484 [details]
Bug 1380927 - Fix context menu alias;

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

Comment 4

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

Errors, above solved offline.

@Ricky, can you please test the following:

- Apply the patch above
- Run the net monitor in the Launchpad
- Open DevTools on the Launchpad page and see if the generateod netmonitor.js bundle contains `let { top } = menuItemNode.getBoundingClientRect();` line in `showsSubMenu` method.

This line represents the change introduced in `devtools-contextmenu` module (v0.0.3), but from some reason it isn't included in the result bundle (netmonitor.js)

Honza

Comment 5

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

@Jason: There is a new version of devtools-contextmenu module, but from some reason it isn't included in the final Netmonitor bundle when building it using Webpack.

Do you have any tips why?

STR:
- Apply the attached patch
- Go to devtools/client/netmonitor directory
- run yarn install & webpack
- Check out the bundle file: assets/build/netmonitor.js - it doesn't contain this change: https://github.com/devtools-html/devtools-core/commit/8b0e606922ddd9fcb6a3f9bc18c6f4f6f9b97c10


Honza

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Can you try to upgrade devtools-launchpad to the latest version and see what happen?

Comment 7

[Jason Laster [:jlast]]

yep, the launchpad should be updated.

https://github.com/devtools-html/devtools-core/blob/master/packages/devtools-launchpad/package.json#L59-L60

For launchpad dependencies, we should upgrade them both at the same time.

Comment 8

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8886484 [details]
Bug 1380927 - Fix context menu alias;

https://reviewboard.mozilla.org/r/157280/#review162736

Please upgrade devtools-launchpad.

Comment 9

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

Comment on attachment 8886484 [details]
Bug 1380927 - Fix context menu alias;

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

Comment 10

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8886484 [details]
Bug 1380927 - Fix context menu alias;

https://reviewboard.mozilla.org/r/157280/#review163262

Thanks Honza. Your patch looks great and it fixed the position issue in sub-menu.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c5e8ab69906d
Fix context menu alias; r=rickychien

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c5e8ab69906d