29400 - Menus don't work properly when too many submenus are open

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Using build 2000-02-26-08 on Win98SE

Steps to reproduce:
1. Open the View menu
2. Open the "Character Coding" submenu
3. Open the "ISO" submenu
4. Place mouse over the "Translate" menuitem in the View menu

Actual resaults:
The "Translate" item only gets temporarly selected and the other submenues are 
still open

Expected resaults:
The submenues should close and the "Translate" submenu shoud be opened

This is even more interesting when if you open
Bookmarks->Sample SmartFind queries->Find: Bookmark Names starting with 'A'
and then slides over to "Personal Toolbar Folder". But do only very small 
movements over the "Personal Toolbar Folder" item.

Comment 1

[Mike Pinkerton (not reading bugmail)]

works for me...not sure what to say.

Comment 2

[sairuh (rarely reading bugmail)]

oh, i see what [e-mail] means. able to reproduce this using the opt
comm bits [2000022808] on both winNT and linux. (not an issue on mac.)

1. select View > Character Coding > ISO. do this with a single click; don't keep
mouse button held down, but make sure the menu remains up w/ISO highlighted.
2. do a mouseover to View > Translate.

expected: Character Coding > ISO should no longer be selected (should go away),
and Translate should become selected (highlighted).

results: Translate is momentarily highlighted, but focus goes quickly goes back
to Character Coding > ISO (which remains up). don't even have a chance to click
on Translate so that it'd stay up...

Comment 3

[Mike Pinkerton (not reading bugmail)]

as i said before, this works just dandy in my non-commercial debug build from 
this morning on NT. can't try linux now, it's busy doing mem leak checking for me 
right now.

bizarre....

Comment 4

[Mike Pinkerton (not reading bugmail)]

you did all the timer code recently, can you take a look at this for me?

Comment 5

[sairuh (rarely reading bugmail)]

tried this out using the non-comm (but optimized) bits [2000022808] on
winNT...and it's still a problem for me.

Comment 6

[David Hyatt]

I can easily reproduce this on Win98 debug build.  I know what's going on too.  
Fix is easy.

Comment 7

[Matthew T (active 1999-2002)]

Reproduced on build 2000032308, MacOS 8.6, Bookmarks menu (Personal Toolbar). 
Platform/OS = All.

Comment 8

[sairuh (rarely reading bugmail)]

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

Comment 9

[:deanis74]

Attachment: Fix attached. First time I've taken out code because there was too much of it.

Comment 10

[Mike Pinkerton (not reading bugmail)]

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

Comment 11

[Peter Trudelle]

Mass-moving all M16 non-feature bugs to M17, which we still consider to be 
part of beta2

Comment 12

[David Hyatt]

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

Comment 13

[:deanis74]

Anybody care to throw the "patch" keyword on here?  I don't have permission to.

Comment 14

[Mike Pinkerton (not reading bugmail)]

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

Comment 15

[Mike Pinkerton (not reading bugmail)]

cool. i'll try the patch today. sorry dean, you know we're in a timewarp here ;)

Comment 16

[:deanis74]

I understand completely, Mike.  I'm just doing my best to let people know when I 
(think I) have fixed something.

Comment 17

[Blake Ross]

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

Comment 18

[Mike Pinkerton (not reading bugmail)]

i'll take this since hyatt is gimpy.

dean, i tried your patch and unfortunately is it a bit _too_ eager. It fixes the 
case in the bug, but then it also closes up the submenu when moving diagonally 
from the parent item into the submenu when the timer fires. We need to leave the 
check in there and probably write some more code (instead of removing code) to 
handle that case.

i'm going to try to get to it in the next day or so myself. Thanks (at very 
least) for pointing out exactly where the the problem lies.

Comment 19

[:deanis74]

Damn, thought I had it there.  If you want some help looking at this, I should 
probably be able to help once I look at that drag-and-drop code.

Comment 20

[Peter Trudelle]

moving to m18

Comment 21

[:deanis74]

Is M18 before beta2?  If not, I think that push back should be re-considered.  
Sure, this is a UI problem, but it's a very noticeable UI problem.

Comment 22

[Mike Pinkerton (not reading bugmail)]

removing keywords: not pp, patch not quite right.

Comment 23

[sairuh (rarely reading bugmail)]

dean, m18 is post-beta2, afaik.

if it ain't, do inform me/us :-)

Comment 24

[Robert Hutchinson]

A (very) minor difference in this bug I found (build 2000052015, win95): 

Tasks -> Privacy and Security -> Password Manager, then mouseover specifically 
to Tools (another menu option with a submenu). 

Expected: All Privacy and Security submenus close, and Tools submenu opens. 

Results: If the navigation is done with a click-and-drag, the Privacy and 
Security submenus close and the Tools submenu opens, but Tools is only 
highlighted for a moment before the highlight moves back to Privacy and Security 
(the submenus behave properly, but the submenu titles do not). A slight mouse 
movement moves the highlight back to Tools, and keeps it there. 

This also happened about half the time with a click-and-release navigation; the 
other half of the time, the Privacy and Security submenus stayed open and the 
Tools submenu failed to open just as described in the main bug report. One 
slight mouse movement then usually closed and opened the proper submenus, but 
didn't properly transfer the highlight to Tools. Another slight mouse movement 
did. 

Comment 25

[Mike Pinkerton (not reading bugmail)]

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

Comment 26

[Peter Trudelle]

mass-moving all bugs to m21 that are not dofood+, or nsbeta2+

Comment 27

[sairuh (rarely reading bugmail)]

*spam*: transferring current XP Menu bugs over to jrgm, the new component owner.
feel free to add me to the cc list (unless am the Reporter) of any of these, if
you have any questions/etc.

Comment 28

[Mike Pinkerton (not reading bugmail)]

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

Comment 29

[Mike Pinkerton (not reading bugmail)]

definately want to fix this for beta3

Comment 30

[:deanis74]

I'll try and take a look at this again tonight or tomorrow night.  It's been a 
while, so I may be completely lost when I get into the nitty gritty.

Comment 31

[Mike Pinkerton (not reading bugmail)]

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

Comment 32

[:deanis74]

I pulled fresh and my build failed in the plugin directory.  I'll try again 
tonight.

Comment 33

[:deanis74]

Is there a function to tell if a menu item is selected?  I can find 
IsDisabled(), IsChecked(), etc., but no IsSelected().

Comment 34

[:deanis74]

Forget it.  I think I was looking for the menuActive nsXULAtom.

Comment 35

[:deanis74]

OK, I know what's happening here.  Let me see if I can fix it...

Comment 36

[:deanis74]

We weren't drilling down far enough into the open sub-menu chain to see if there 
was a selected menu item.  We always just checked for a selected menu item in 
the first-level sub-menu.  We didn't care if that selected menu item also opens 
a sub-menu, so it always assumed that there was something active.  I don't know 
if that makes sense or not, but it's here partially as documentation to me, so I 
know what the heck I was thinking.

I found it easiest to duplicate the current behavior using the View > Character 
Set sub-menu.  Pop out one of the sub-menus under there, and then try to 
highlight a different item in the View menu, like Page Source.  Before putting 
in my fix, the highlight will jump back to the Character Set menu item and all 
of the sub-menus remain open.  After applying my fix, it works as expected.  At 
least for me, that is.

Man, I hope this is it, because this is a little bug that sure causes a lot of 
annoyance (at least 8 dupes).

Comment 37

[:deanis74]

Attachment: I think I may just have nailed this one. Can someone check this for me?

Comment 38

[:deanis74]

Sorry for the spam - adding the "patch" keyword.

Comment 39

[:deanis74]

Attachment: Minor change - comments were wrong. The more I use this the more I'm sure I've nailed it.

Comment 40

[Mike Pinkerton (not reading bugmail)]

fixed, thanks dean!!! I also added a comment at the head of Notify() so that 
people might actually be able to understand what the heck is going on there.

Comment 41

[:deanis74]

Yeah, that's what I was trying to say...  ;-)

Comment 42

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

verifying. This works perfectly. tnx