138826 - context menu problems in stand alone message window: "Open Link in New Window" doesn't work, too many items are showing up, etc.

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Brook Harty]

Right mouse menu "Open Link in New Window" broke in Opened email only.
It does work correctly in email preview mode. (non-windowed)

1. In email, double click and open an email in its own window.
2. Try to open a link with the right mouse menu "Open Link in New Window"
Doesnt work.



Works correctly on 0.9.8 on my linux box.
Broken on RC1 and 0.9.9 for the last week on both linux/windows version. Last version checked 2002041803.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Yep.  "gContextMenu has no properties" is the error in the JS console.

Comment 2

[R.K.Aa.]

dup of bug 133334?

Comment 3

[Antonio.Xu]

i will research this bug

Comment 4

[Antonio.Xu]

Attachment: This is a new patch for this bug,please rv

i think when message window popup a menu,it should same as messagepanebox in
mail3PaneWindowVertLayout.xul.so i change the contextmenu.please rv my patch.

Comment 5

[David :Bienvenu]

Comment on attachment 80747 [details] [diff] [review]
This is a new patch for this bug,please rv

r=bienvenu

Comment 6

[R.K.Aa.]

*** Bug 141347 has been marked as a duplicate of this bug. ***

Comment 7

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

Comment on attachment 80747 [details] [diff] [review]
This is a new patch for this bug,please rv

needs work, this patch does work for me.

Comment 8

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

over to ssu.  can you investigate?

Comment 9

[Sean Su]

yeah, I can take a look at it.

Comment 10

[Antonio.Xu]

I had researched this patch,it is worked for me.

Comment 11

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

antonio: I just tried the fix again on the trunk, and it does not work.

Comment 12

[Boris 'pi' Piwinger]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0+) Gecko/20020503

Even though the context menu does not work, the middle mouse button does. So
some people will have a work-around.

pi

Comment 13

[Andrew Hagen]

*** Bug 133334 has been marked as a duplicate of this bug. ***

Comment 14

[Andrew Hagen]

Moving regression keyword and major severity over from duplicate bug. Adding
Mozilla1.0 keyword.

When did this regression occur?

Comment 15

[R.K.Aa.]

the "new series" of this was first reported on March 25th.
An older occurance was bug 91720.

Comment 16

[Michael Buckland]

Discussed in mail news bug meeting with Engineering Marketing and PjM.  Decided
to plus this bug.

Comment 17

[Antonio.Xu]

it is a important bug,i have researched it,i found messagewindow use error menu 
context,so i think if we use the right menu context,we can fix it.

Comment 18

[Antonio.Xu]

Attachment: This is a new patch for trunk,it can fix this bug in RC2 and trunk

forget my frist patch,i have researched this bug again,i found when pop menu
set up,it will get menuitem like that
var menuitem = document.getElementById("context-viewbgimage");
but menuitem maybe is null,if it is null,we should execute
menuitem.removeAttribute("disabled");
or
menuitem.setAttribute("disabled", "true");
please rv and sr my patch,thank you

Comment 19

[Antonio.Xu]

sorry
we should not execute
menuitem.removeAttribute("disabled");
or
menuitem.setAttribute("disabled", "true");

Comment 20

[Peter Doyle]

*** Bug 143608 has been marked as a duplicate of this bug. ***

Comment 21

[Antonio.Xu]

please rv &sr my patch.

Comment 22

[Jonas Jørgensen]

*** Bug 143506 has been marked as a duplicate of this bug. ***

Comment 23

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

important for two pane users, who rely on the stand alone msg window.

good work, antonio.  but this fix is not complete, there are still context menu
problems in the stand alone window.

here comes a complete patch...

Comment 24

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

Attachment: complete patch.

also fixes the problem with context menu for images in the stand alone msg
window.

Comment 25

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

Attachment: patch, plus some other code clean up

Comment 26

[Blake Ross]

Comment on attachment 84814 [details] [diff] [review]
patch, plus some other code clean up

sr=blake except for the nsDownloadProxy.h change; I don't see why that's a good
change to make?

Comment 27

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

> except for the nsDownloadProxy.h change; I don't see why that's a good change
to make?

you're right, not necessary.  I was being a style nit picker.  Since it's your
code, I'll leave it alone.

fixed on the trunk.  the nsContextMenu.js fix is something low risk, high reward
that we want for the branch after 1.0 ships.

Comment 28

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

note to qa:

when verifying, test the context menu for the message, and the context menu for
images within the message.

Comment 29

[[e-mail]]

Bah. (See bug 137141.)

Comment 30

[scottputterman]

removing the rtm nomination for the moment.  Can we get a review on this in
addition to the super review.  After that happens, just add the adt1.0.0
nomination back.

Comment 31

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

> Bah. (See bug 137141.)

For the end user, is there something your patch for #137141 does, that this 
patch doesn't?

If not, then let's just go with this patch on the 1.0 branch and let yours go 
onto the trunk.  (Sorry for the conflicts you'll get when you update, as my 
patch has already landed on the trunk.)

Comment 32

[David :Bienvenu]

Comment on attachment 84814 [details] [diff] [review]
patch, plus some other code clean up

r=bienvenu

Comment 33

[[e-mail]]

Comment on attachment 84814 [details] [diff] [review]
patch, plus some other code clean up

Does this patch fix the case where text is selected? My patch also checks
whether the search for selected text menuitem exists, I don't see that in this
patch.

Comment 34

[Olga]

Tested on today's trunk build: a message with text, a link, an image in Stand
Alone window.
'Open in New Window' works, but not expected multiple group's separators are there.
Spec: http://www.mozilla.org/mailnews/specs/threepane/MailMenus.html#Context 

Comment 35

[scottputterman]

Olga, what are you referring to?  Could you provide a screenshot?

Comment 36

[Jaime Rodriguez, Jr.]

Removing adt1.0.0, and adding adt1.0.1, as we have changed milestones.

Comment 37

[R.K.Aa.]

see bug 148078 and bug 148561
A screenshot in attachment 85642 [details]

Comment 38

[scottputterman]

http://bugzilla.mozilla.org/show_bug.cgi?id=148561 is reported as an RC3 bug. 
This  fix is checked onto the trunk only, so RC3 wouldn't have it.

reassigning to ssu.  Sean, can you figure out the state of this patch on the
trunk and look into if there's anything we need to fix before we can check into
the branch?

reopening based on comments until we've investigated this.

Comment 39

[scottputterman]

actually, not reassigning if antonio can still look into this.  But, we need
this fixed, so Sean can you still help look into this.

Comment 40

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

> see bug 148078 and bug 148561

this patch is safe for the branch.  the problem is that this has since been 
regressed on the trunk.

bug http://bugzilla.mozilla.org/show_bug.cgi?id=138018 caused the regression.

I'll work on the trunk regression in bug 148078 and bug 148561

re-assign to ssu, so he can verify this is safe on the branch, and land it once 
we get approval.

Comment 41

[Syd Logan]

adt1.0.1+ on behalf of ADT

Comment 42

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

http://bugzilla.mozilla.org/show_bug.cgi?id=148078 has been fixed on the trunk.

Comment 43

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

> Bah. (See bug 137141.)

I think ssu is going to look into landing that on the branch, instead of this 
patch, as that bug contains this fix as well.

Comment 44

[Sean Su]

marking this fixed cause bug 131141 is fixed on trunk.

Comment 45

[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 46

[Sean Su]

bug 137141 has been checked into the branch, so marking this bug fixed on the
branch as well.

Comment 47

[Olga]

Verified on branch 06/12/02, Win2K (classic), Linux (modern), Mac OSX (modern).

Comment 48

[Brook Harty]

Verified it works in 1.0.1 nightly build. 
Question, wouldnt you want to include "Open Link in New Tab" also? Or is there a seperation between browser and email client?

Comment 49

[Matthias Versen [:Matti]]

*** Bug 156969 has been marked as a duplicate of this bug. ***

Comment 50

[R.K.Aa.]

*** Bug 160551 has been marked as a duplicate of this bug. ***

Comment 51

[laurel]

*** Bug 172621 has been marked as a duplicate of this bug. ***