195878 - "Open Link ..." in context menu does not work if "This Frame" submenu was opened.

Status: VERIFIED FIXED

(SeaMonkey :: General, defect)

Description

[u69748]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3) Gecko/20030301
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3) Gecko/20030301

"Open Link ..." in context menu does not work if "This Frame" submenu was opened.

Reproducible: Always

Steps to Reproduce:
1. Go to a page which has frame.
   Ex. http://www.zvon.org/tmRFC/RFC2822/Output/
2. Right click a link. Then context menu is opened.
3. Move mouse pointer on "This Frame". Then submenu is opened.
4. Click "Open Link in New Window".

Actual Results:  
Nothing happens.



I can reproduce this on Netscape7.02, 1.2.1 and latest-trunk

Comment 1

[Patrick]

I'm not sure if i understand your problem correctly. So please correct me if i'm
wrong. What you mean is that 

1) For a framed page "Open link in.." works
2) For a framed page "Open link in.." does NOT work after the context-submenu
THIS FRAME has been viewed.

So in your steps to reproduce, you do a step 3a, return to the first context-menu.

Indeed, the behavior you describe reproduces on my machine XP Pro SP1.

Comment 2

[u69748]

Patrick, your are correct.

Steps to Reproduce:
1. Go to a page which has frame.
   Ex. http://www.zvon.org/tmRFC/RFC2822/Output/
2. Right click a link. Then context menu is opened.
3. Move mouse pointer on "This Frame". Then submenu is opened.
4. Move mouse pointer on "Open Link in New Window".
   Then submenu is closed.
5. Click "Open Link in New Window".

Comment 3

[Ian Nartowicz]

I believe that you are describing the behaviour of bug 21390, where the first
click just closes an open menu (or submenu) and the second click actually does
something.  I thought this was fixed and I can't reproduce it on Win98.  For me,
the submenu closes as soon as I mouse over the main menu.  Maybe XP-specific? 
Also see bug 66834 for a more general discussion of when clicks should or
shouldn't operate.

Comment 4

[MrJeff]

I get this problem too in windows 2000.  Ian Nartowicz, you don't have to go to
the main menu, simply open the submenu and then click on the "open link in new
window" on the popup menu you already have.

Is probably worth noting that many but not all of the menu items don't work (it
seems to be all the ones that want to try to open a new window). Open link in
new tab, and properties don't work. Select all does.

Comment 5

[Jesiah S]

I see this problem too on Linux 2003030308 and on my day old CVS GTK2/XFT build

Comment 6

[Benedikt Kantus]

So this cannot be a dupe of bug 21390 (commetn #3), because some menu item work
(comment #4).

Comment 7

[Boris Zbarsky [:bzbarsky]]

I get "gContextMenu has no properties" when this error happens.

Over to a real person; this is quite major (I bet someone tears down
gContextMenu onpopuphide, but it's the wrong popup!).  This could fix a whole
slew of reports we have of context menus sort of "not working".

Comment 8

[[e-mail]]

Attachment: Proposed patch

Comment 9

[jag (Peter Annema)]

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

For symmetry you might want to use the |event.target == this| check in both
places (instead of the !gContextMenu check).

Though... If we add the |event.target == this| check to onpopupshowing, do we
still need to set gContextMenu to null in onpopuphiding?

Comment 10

[[e-mail]]

Interesting... this from bug 73804:

------- Additional Comment #7 From Blake Ross  2001-10-10 13:35 -------

The real fix is for onpopuphiding and onpopupshowing to not bubble

Comment 11

[jag (Peter Annema)]

Or perhaps onpopupshowing/-shown/-hiding/-hidden should trigger for
phase="target" instead of phase="bubble" (though that would be weird, since
everything else triggers for the latter, even though a lot of people seem to
think of it as the former).

Neil, what do you think about my suggestion to check event.target in both places
and remove the null check/setting? My reasoning behind it is that it'd make the
code easier to understand since there's no interaction between showing/hiding
that way.

Comment 12

[[e-mail]]

OK, so how about this:

if (event.target != this) return true;
gContextMenu = new nsContextMenu(this);
return gContextMenu.shouldDisplay;

Comment 13

[jag (Peter Annema)]

That would work too. Pick which one you think is best and attach a new patch.

Comment 14

[[e-mail]]

Attachment: Always check event.target

Comment 15

[[e-mail]]

Comment on attachment 121515 [details] [diff] [review]
Always check event.target

Low risk fix.

Comment 16

[Asa Dotzler [:asa]]

Comment on attachment 121515 [details] [diff] [review]
Always check event.target

a=asa (on behalf of drivers) for checkin to 1.4b.

Comment 17

[[e-mail]]

Fix checked in.

Comment 18

[Neil Marshall]

Can someone check to see if this patch fixes bug 145585 ?

Comment 19

[Wesha]

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

Comment 20

[Wesha]

Confirmed fixed 2003042508/NT4

Re: comment 18:  yes it does; marked as dupe.