Akkana pointed out that the context menu should be made more global. This is in nav.xul <script language="JavaScript" src="chrome://navigator/content/nsContextMenu.js"/> This is needed so context menu's will work in the embedding case.

nom nsbeta2

Putting on [NEED INFO] radar. PDT needs to know impact to user and risk of fix to make a call on this bug.

Putting on [nsbeta2-] radar. Not critical to beta2.

no, not critical to nsbeta2, but critical for putting a xul window inbetween teh docshell and the webbrwoser for emebedding (hyatt or adam correct me if I'm wrong).

removing the minus for reconsideration.

Deferring until some type of decision is made on the embedding issues being discussed by pdt and judsen.

As far as I can see, this class is VERY specific to the navigator and has little to reuse except for some helper methods. Perhaps these could be split out? The class itself is created when the popup menu is about to be displayed. The constructor hides/shows and enables/disables menu items in the popup depending on the current context. The class calls into lots of globals to figure out what is the context exactly. Popups work without such a complicated class, but obviously if you need to grey out things or whatever, then you need to run some code before the popup is shown to do all this.

Marking invalid because this class is not generic and can't be shared.

Reopening. I believe akkana's original point was that this .js class was pulled into editor.xul. (It's also pulled into EditorContextMenuOverlay.xul) I looked at the editor use of this class, and it appears that it is created and then never used (but I could be wrong). At any rate, if there are common methods that are used, these should be refactored into a base implementation; if they aren't used, then this linkage between editor and navigator should be broken (don't pull in nsContextMenu.js if it isn't used).

I agree. Can we bounce this over to an editor person for analysis?

I think that might be brade ... does Composer really use nsContextMenu.js for its context menus, and, if it does, what common methods could be factored out to a utility class. (cc: law who owns nsContextMenu.js).

mail also uses nsContextMenu.js It's been a while since I looked at this code; I'll investigate a bit later today.

The editor references to nsContextMenu.js aren't really ever used and can be safely removed. I have fixes which I can check in if this is nsbeta2+. I'd recommend a mail person investigate their uses of nsContextMenu.js and see if they are truly needed there. (wish I knew who to cc)

cc putterman for mail use of nsContextMenu.js

nsContextMenu.js was intended to be a "utility" in and of itself. It needs a little work in order to function better in that role, probably. I think we just need to move it to a more global location for now. The actual context menu xul within navigator.xul is theoretically independent of the code in nsContextMenu.js (making it more independent is the biggest obstacle to making the JS more utilitarian). I don't think other components are using that xul, however, so I don't think navigator.xul needs to "go to communicator."

Yes, we use it. We use it in the message pane when someone right clicks on html mail. right now it works. I probably should have tried making it more generic when I implemented it but I was trying to make feature complete and I could make it work the way it was. If we break it out then I could change our code to use the new version. As far as I know we use nsContextMenus.js and navigator.js (the big win would be not importing all of navigator.js).

Putting on [nsbeta2-] radar. Valeski will change to + when he sees fit.

assigning to valeski

plussing and over to brade for her changes, then back to mailnews.

setting to m18 to get on brade's radar

at this time, Composer doesn't reference/use nsContextMenu.js; reassign to putterman for mailnews fixes

It's unclear to me why it's an nsbeta3+ for me to break out common code in navigator.xul and nsContextMenu.js for mailnews' usage. If it were to be broken out, it would make it easier for mailnews and others to use it, but the last I checked it seemed to be working.

it's beta3+ because our embedding clients would like to start using some XUL without having to recreate logic.

Let me rephrase my question. Why is this assigned to me? :-)

Judson, we won't do this for Mail in beta3. I'm sending it back to you to own or reassign to the nav team. If you can plus it and fix it, go for it.

->pinkerton

I'm a little confused why this is on my plate. nsContextMenu.js is owned by XPApps.

Embedding needs context menus that aren't owned by XPApps.

please add swag to status whiteboard

don't know what's involved, so this a totally wild guess

seems like a dup of 59707. alec and I are working on it. *** This bug has been marked as a duplicate of 59707 ***

Updating QA Contact

Verified as Duplicate