137141 - Wrong context menu items in stand alone mail window

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Lasse Marøen]

The stand alone mail window has a different context menu than the one in the
3-pane window. More specifically it includes items for images, event if the mail
doesn't contain images. 

To reproduce:

1. Open a folder in MailNews
2. Double click a message to open in a seperate window
3. Right-click on an empty area in the mail window
4. Select "save image as"

Expected result:

The same menu as in the 3-pane window, no options for images, links, etc.

Was this simply forgotten in bug 75338? 
Assigning to Blake Ross, since he had bug 75448. 

WinXP, 2002041103, trunk.

Comment 1

[Lasse Marøen]

> Assigning to Blake Ross, since he had bug 75448. 

Obviously, I meant to say bug 75338.

Another thing, I just noticed that the window for "view message source" has a
context menu that looks like that of a browser window. Kind of the same thing,
so I'll mention it here instead of opening a new bug. 

Now off to check context menus in all windows..

Comment 2

[Adam Macznik]

Just a note, that when (if) this one is fixed - so the context menus in
standalone window will match those in 3pane - there are some bugs which will
probably be resolved:
Bug 86744 - Stand Alone Context Menu: Image - Not Link
Bug 86741 - Stand Alone Context Menus: Part or All of Context Area, link
Bug 86743 - Stand Alone Context Menus: Part or All of Context Area, Image
Bug 86748 - Stand Alone Context Menus: Image Link
Bug 72266 - Stand Alone and 3-pane win: Context Menus: Nothing particular is
selected
Maybe those bugs should be reviewed and made dependent on this bug or v-v?

Comment 3

[[e-mail]]

Attachment: Proposed patch

Comment 4

[[e-mail]]

The actual problem is that nsContextMenu.js doesn't do enough error checking.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 84416 [details] [diff] [review]
Proposed patch

r=biesi

Comment 6

[[e-mail]]

Attachment: Updated for biesi's comments

Changed true to "true" and added some ()s

Comment 7

[[e-mail]]

Comment on attachment 84423 [details] [diff] [review]
Updated for biesi's comments

Transferring biesi's r=

Comment 8

[Sean Su]

Attachment: neil's patch updated to tip

This is essentially neil's patch, but updated to the recent tip because it got
bit rotten.

I've also fixed a couple of menuitem separator problems in the message pane
context menu (since neil's original patch deals with this particular context
menu anyways).	This particular fix is in mailnews' mailContextMenus.js.

Neil could you take a quick look at this updated patch and provide r=?

Comment 9

[Sean Su]

reassigning to myself to make sure it gets properly merged into the tip and trunk.

Seth, could you provide sr=?

Comment 10

[(not reading, please use [e-mail] instead)]

Comment on attachment 86313 [details] [diff] [review]
neil's patch updated to tip

sr=sspitzer, looks good.

please test well.

links and images and selecting text (and then doing context menus)

test news, pop, imap messages, (and special shared imap message, see
http://bugzilla.mozilla.org/show_bug.cgi?id=138018)

and test windows and linux, since some of this code (set wall paper) is win
only.	

when you test linux, do you see what R.K.A. is seeing in
http://bugzilla.mozilla.org/show_bug.cgi?id=148078#c10

your fix mailContextMenu.js might fix what he is seeing.

Comment 11

[[e-mail]]

Thanks for fixing the bitrot but unfortunately no changes to mailContextMenus.js
can fix the block/unblock problem you're seeing because the code that
shows/hides those items executes after the code in mailContextMenus.js; instead
you probably need to change the false at the end of line 111 of
cookieContextOverlay to true.

Comment 12

[Sean Su]

Comment on attachment 86313 [details] [diff] [review]
neil's patch updated to tip

nope, that does not fix it.

Assuming r= from neil for the patch in its current state since his alternate
suggestion does not fix the problem.

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 86313 [details] [diff] [review]
neil's patch updated to tip

a=asa (on behalf of drivers) for checkin to the 1.1 trunk.

Comment 14

[Sean Su]

fixed checked in to trunk only.  seeking branch approval now.

Comment 15

[[e-mail]]

Ah, I'd forgotton that there's another bug that means that the order of multiple
event listeners on the event target is always the order that they are added,
[despite using capturing event listeners] so any overlay that gets added to the
context menu will break it horribly! Since cookieContextOverlay.xul breaks the
separator code it should be fixed there rather than in mailContextMenus.js

Comment 16

[[e-mail]]

omigod context menus are so ugly. I just looked at the 3pane in DOM inspector;
it contains no fewer than three pairs of imageblocking menuitems, one pair in
the message pane context and two pairs in sidebar's content area context: the
imageblocking menuitems get overlaid twice on to the content area context menu.

So a belated r= on ssu's patch because one lame hack deserves another :-)

P.S. the context menu code assumes the existence of gContextMenu defined in
contentAreaContextOverlay.xul which isn't overlaid by the message window...

Comment 17

[Jaime Rodriguez, Jr.]

Resolving as fixed, because this has landed on the trunk. Adding Mozilla1.0.1,
and adt1.0.1 for approval to land on the 1.0 branch.

olgam - can you pls verify this on the trunk. thanks!

Comment 18

[Judson Valeski]

please checkin to the 1.0.1 branch. once there remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 19

[Sean Su]

patch finally checked into branch.

Comment 20

[scottputterman]

removing adt1.0.1 to get off of the radar.  You need an adt1.0.1+ to check into
the branch.

Comment 21

[Olga]

Verified on branch 06/12/02, Win2K (classic), Linux (modern), Mac OSX (modern).
Stand Alone message components have the same context menu as a message in 3-pane.
Tested with POP, IMAP, newsgroup messages comparing 3-pane and Stand Alone
context menus for:
    - blank area
    - text
    - image
    - link
    - image-link
P.S. Also checked and updated bug 148078.
Bugs mentioned in comment #2 are still valid.