Opened 7 years ago

Last modified 7 years ago

#9576 assigned bug

If a window closes while the user is CLICKING on a menu, the menu is still there

Reported by: waddlesplash Owned by: nobody
Priority: normal Milestone: R1
Component: Kits/Interface Kit Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Example: open a .zip file with Expander, hit the button to start extracting, and then move the mouse and CLICK on a menu and HOLD THE MOUSE DOWN. After Expander finishes and the window disappears, the menu will still be there (!!). Releasing the mouse, the menu is still there. If you had clicked on the "File" menu and move your mouse to the right, you will get the settings menu (still floating in midair). Until you move your mouse away from where the menu was/is, the floating menu will still be ther.

Attachments (2)

menutester.cpp (1.6 KB ) - added by jscipione 7 years ago.
A small program to test this bug.
MenuTester.zip (8.0 KB ) - added by jscipione 7 years ago.
Compiled MenuTester application

Download all attachments as: .zip

Change History (18)

comment:1 by anevilyak, 7 years ago

Component: User InterfaceKits/Interface Kit
Owner: changed from stippi to axeld
Version: R1/alpha4.1R1/Development

comment:2 by jscipione, 7 years ago

Owner: changed from axeld to jscipione
Status: newassigned

What is the correct behavior, should the window not be allowed to close (as it works on Windows) or should the menu be closed with the window (as it works on Mac OS X)?

comment:3 by waddlesplash, 7 years ago

Hmm... What does BeOS do? I don't have a working BeOS VM, sorry. But I think it makes more sense the menu closes with the window. Some apps may start behaving strangely when they ask the window to close and it doesn't.

comment:4 by jscipione, 7 years ago

BeOS R5 has identical behavior to Haiku as described in this bug report. The window closes while the menu remains open.

comment:5 by jscipione, 7 years ago

Since pushing Cmd+W does not close a window when you have a menu open I think that the correct behavior should be to not allow the window to close programatically either.

by jscipione, 7 years ago

Attachment: menutester.cpp added

A small program to test this bug.

by jscipione, 7 years ago

Attachment: MenuTester.zip added

Compiled MenuTester application

in reply to:  5 ; comment:6 by axeld, 7 years ago

Replying to jscipione:

Since pushing Cmd+W does not close a window when you have a menu open I think that the correct behavior should be to not allow the window to close programatically either.

While that sounds right, I don't think it makes any sense. What should happen on Quit() in that case? Of course, the window could just block until the menu is open, but what's the point? It's not like the menu action will do anything anymore.

I think the best behaviour would indeed be to close the menu with the window. Each menu has an owning window, so this shouldn't be much of a problem to implement either.

in reply to:  6 comment:7 by jscipione, 7 years ago

Replying to axeld:

I think the best behaviour would indeed be to close the menu with the window. Each menu has an owning window, so this shouldn't be much of a problem to implement either.

I discussed this a bit more on IRC and it seems like an even better behavior would be to not close the window if you send a B_QUIT_REQUESTED message (if you ask the window to quit) but to close the menu and window if you call the Quit() method (you tell it to quit).

comment:8 by axeld, 7 years ago

I just wonder what good it would do. The window would have no chance to react to the menu item you'll choose either way, as B_QUIT_REQUESTED will be the last thing it'll do. As I said, it doesn't make any sense.

comment:9 by axeld, 7 years ago

And no, you cannot change QuitRequested() to return false in this case, as this would be a major API change, and, from the point of the developer, completely random behavior.

in reply to:  9 comment:10 by jscipione, 7 years ago

Replying to axeld:

And no, you cannot change QuitRequested() to return false in this case, as this would be a major API change, and, from the point of the developer, completely random behavior.

Why can't I change BWindows's QuitRequested() method to return false? How is that a major API change, how is that completely random behavior? AFAIK the BWindow::QuitRequested() is allowed to return false to indicate that quit was cancelled and the caller is suppose to check the return value and respond appropriately (say by manually Quit()ing instead).

I have looked at the BWindow quit and deconstruction code as well as the menu tracking code for several hours now and it is a bit hairy with a few hacks piled up over the years and I'm a bit reluctant to mess with it too much. I'm looking for the easiest way to solve this bug without breaking anything. The correct behavior, I think, is to cancel closing the window, but, if it is easier to close the menu instead I'll do that. So far I haven't found an easy way to do either.

comment:11 by waddlesplash, 7 years ago

I say close the window. Because right now if a menu is open, that's what happens. Only if the user is holding the mouse down does the window not get closed.

comment:12 by axeld, 7 years ago

What BWindow::QuitRequested() does lies in the hands of the developer, not the user of the application. If posting B_QUIT_REQUESTED to a window all of a sudden does not do as expected, it's random and untestable behavior from the POV of the application developer.

Furthermore, you cannot change QuitRequested() simply for the reason that you usually not call it when you develop an application either; you'll just overwrite it when you need it to behave differently than always returning true.

Both together make it a major API change, that is even completely undesirable.

comment:13 by jscipione, 7 years ago

Okay, but, what if I were to block closing the window while I waited for the user to release the mouse button? I wouldn't have to touch QuitRequested() at all.

comment:14 by jscipione, 7 years ago

Owner: changed from jscipione to nobody

Someone interested please take this one.

comment:15 by jackburton, 7 years ago

If I remember correctly (but probably I don't), the lines

        //Wait if a menu is still tracking
	if (fMenuSem > 0) {
		while (acquire_sem(fMenuSem) == B_INTERRUPTED)
			;
	}

in BWindow's destructor are the ones which waits for the BMenu to finish tracking.

I can't remember the bug I was trying to fix in commit cb8bdc4edae377c5900904a38f62c0ec08d5c5a5, but might be worth reverting to see what happens.

in reply to:  5 comment:16 by leavengood, 7 years ago

Replying to jscipione:

Since pushing Cmd+W does not close a window when you have a menu open I think that the correct behavior should be to not allow the window to close programatically either.

Cmd-W or other shortcuts not working while a menu is open is a side effect of the menu implementation, where the open menu eats all the keyboard input, even though in theory it should be perfectly fine to use a shortcut like Cmd-W while a menu is open and have it be directed to the owning window (it works in Mac OS X.)

I'm pretty sure I looked into fixing this once but as you've seen the menu code is a mess, and very hard to understand.

This bug is probably also difficult to fix cleanly, and I agree the best solution is to close the menu when the owning window closes.

I've wanted to just scrap the menu code and try starting over (Stephan has had a similar idea too) but it would help to have some sort of automated or semi-automated test suite first. The API design makes it difficult to fix cleanly, because BMenuBar is a menu, and BMenuField uses a menu, BMenus can contain other BMenus, then there is BPopupMenu, etc. When you fix one, you can break another, as I'm sure jackburton can attest to.

But there are many menu bugs and just bad UI because of this implementation, and it is an important part of the user experience so it might be worth spending the time on to fix it.

At this rate we can't just say we'll wait until after R1 and break the API, since R1 isn't really a reality yet.

Note: See TracTickets for help on using tickets.