225431 - iframe context menu does not appear (if menu dismissed with alt or escape key)

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Gyuris Gellért]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6a) Gecko/20031030
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6a) Gecko/20031030

Alt+Tab

Reproducible: Always

Steps to Reproduce:
1. Right click on an iframe and choose 'This frame' from the context menu. The
submenu appears.
2. Switch to an other application by Alt+Tab
3. Switch back to the browser by Alt+Tab
4. You can not get the submenu of 'This frame' mentioned above any more till
restarting the browser.

Comment 1

[Patrick]

wfm xp pro sp1 2003102804.

Is your 20031030 build an official Mozilla download? Is this Firebird?

Comment 2

[Boris Zbarsky [:bzbarsky]]

I can reproduce it with linux trunk build 2003-11-08-05 (vanilla SeaMonkey
nightly).  Simply pressing "alt" while the "this frame" submenu is up takes down
the whole context menu and the "this frame" submenu does not work thereafter.

Comment 3

[[e-mail]]

This appears to apply to any cascading context menu, linux or windows.

Comment 4

[Gyuris Gellért]

The error occur with the following versions:
Mozilla suite official build - mozilla/5.0 (windows; u; windows nt 5.0; en-us;
rv:1.6a) gecko/20031030
Mozilla suite official build - mozilla/5.0 (windows; u; windows nt 5.0; en-us;
rv:1.5) gecko/20031007
Firebird hungarian version -  mozilla/5.0 (windows; u; windows nt 5.0; hu-hu;
rv:1.5) gecko/20031007 firebird/0.7
Mozilla suite unofficial build Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5)
Gecko/20031111 Debian/1.5-2.backports.org.1

The follwing versions are workong correctly:
mozilla/5.0 (macintosh; u; ppc mac os x mach-o; en-us; rv:1.3) gecko/20030312
mozilla/5.0 (windows; u; win98; en-us; rv:1.4) gecko/20030612

Comment 5

[[e-mail]]

Definitely a regression - 1.4 (win95/w2k) works fine.

Comment 6

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 209634 -- someone somewhere is depending on buggy
behavior wrt attr changes....

Comment 7

[Boris Zbarsky [:bzbarsky]]

OK.  The problem is that when the menu comes down, the nsMenuFrame::OpenMenu is
NOT called in this case.  That means that the "open" attribute is not unset and
the menu keeps thinking it's still open.

We could "fix" this by having nsMenuFrame::OpenMenu unset and then reset the
"open" attr in the "open it up" case... Or we could figure out why the menu is
not being properly closed.

Comment 8

[[e-mail]]

Ah, so what you're saying is that the menu code relies on the attribute changed
notification to open a menu, but somehow the ALT dismisses the menu without
removing the open attribute?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Exactly.

Comment 10

[[e-mail]]

Attachment: Proposed patch

So, it turns out that nsMenuBarFrame::DismissChain calls
SetCurrentMenuItem(nsnull) which calls OpenMenu(FALSE) and SelectMenu(FALSE).
However, nsPopupMenuFrame::DismissChain only calls SelectMenu(FALSE).

Comment 11

[Boris Zbarsky [:bzbarsky]]

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

sr=bzbarsky.  Looks reasonable, assuming you've tested and all...

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Menus and popups can have weird regressions sometimes, but this seems like it
ought to be simple enough.  r=dbaron, but try to get testing on multiple
platforms...

Comment 13

[[e-mail]]

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

Simple regression fix, so far tested on RH7.3 and MingW builds.

Comment 14

[Asa Dotzler [:asa]]

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

a=asa (on behalf of drivers) for checkin to 1.6 beta.

Comment 15

[[e-mail]]

Fix checked in.

Comment 16

[Oliver Klee]

Won't this generate a "uninitalized variable" warning?

+          PRBool wasOpen;
+          mCurrentMenu->MenuIsOpen(wasOpen);

Comment 17

[Boris Zbarsky [:bzbarsky]]

It better not -- that's an out param for MenuIsOpen().

Comment 18

[[e-mail]]

Yeah, it's a reference instead of the more obvious pointer.

Comment 19

[Brendan Eich [:brendan]]

Bah.  Pointers for out params, please.  See
http://www.ocallahan.org/mozilla/interfaces.html, to be hosted soon on
mozilla.org under projects/xpcom/.

/be

Comment 20

[Boris Zbarsky [:bzbarsky]]

Brendan, Neil just used an existing function... and presumably decided that in
the interests of his sanity and fixing this for 1.6 it would be better to NOT
rewrite all of the XUL layout code in this patch.  ;)

Comment 21

[Brendan Eich [:brendan]]

I know -- I was just muttering in a resolved bug, in the hope that curmudgeonly
comments might inspire followup bugs.

/be

Comment 22

[Boris Zbarsky [:bzbarsky]]

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