Bug #332
CtrlLib: MenuBar::Execute changes
Status: | Rejected | Start date: | 09/27/2012 | |
---|---|---|---|---|
Priority: | High | Due date: | ||
Assignee: | Miroslav Fidler | % Done: | 100% | |
Category: | CtrlLib | Spent time: | - | |
Target version: | - |
Description
After changes to MenuBar::Execute (r5365) it doesn't work in some cases.
Test case:
1. Open some package from TheIDE (builded after r5365).
2. Right mouse click on some package from packages list and select "Package organizer.." bar.
3. Inside opened "Package organizer" window try to add "New libraries..", for example.
The same "Package organizer" window, but opened from "Project" -> "Package organizer.." bar, allows to add "New libraries..", for example.
Proposed (temporary) solution is to revert the r5365 changes.
History
#1 Updated by Miroslav Fidler almost 12 years ago
- Status changed from New to Ready for QA
- Assignee set to Sender Ghost
Thanks, reproduced and fixed. Please confirm (and close).
#2 Updated by Sender Ghost almost 12 years ago
- Status changed from Ready for QA to Approved
Yes, now it works. Thanks.
#3 Updated by Sender Ghost almost 12 years ago
- File 332_uppsrc.diff added
- Status changed from Approved to New
- Assignee changed from Sender Ghost to Miroslav Fidler
- % Done changed from 0 to 50
But I would like to propose previous behaviour.
Instead of return case for the same owner, just close the menu and allow to open it again. For example, this allows right mouse click on different packages inside packages list of TheIDE, without extra mouse click to close the menu.
#4 Updated by Sender Ghost almost 12 years ago
My patch didn't solve the problem with repeated right mouse click.
Current solution works for the test case, but (new) issue is open for discussion, still.
#5 Updated by Sender Ghost almost 12 years ago
- File deleted (
332_uppsrc.diff)
#6 Updated by Sender Ghost almost 12 years ago
- File 332_2_uppsrc.diff added
I have added the second patch with possible solution for the (new) issue.
#7 Updated by Sender Ghost almost 12 years ago
But might be exists the possibility of stack overflow for the solution (second patch), because of possible long recursion.
The previous menu level checks apply in reverse, which accumulate with each new level.
#8 Updated by Miroslav Fidler almost 12 years ago
Well, the "full" solution would probably had to use some form of PostCallback so that Execute is left before the new Execute is started. Anyway, for now I consider that sort of quirky, as perhaps things would not behave as one expects (that is, that the menu operation is performed before Execute exits).
#9 Updated by Miroslav Fidler almost 12 years ago
- Assignee changed from Miroslav Fidler to Sender Ghost
#10 Updated by Sender Ghost almost 12 years ago
- File 332_3_uppsrc.diff added
- Status changed from New to Patch ready
- Assignee changed from Sender Ghost to Miroslav Fidler
- % Done changed from 50 to 100
I implemented another approach (the third patch), while using destructor with CloseMenu and One container.
At least, this might give another idea how to implement correctly.
#11 Updated by Sender Ghost almost 12 years ago
- File MenuBarExecute.zip added
Also, there is some kind of simple application to test MenuBar::Execute (for single menu).
#12 Updated by Sender Ghost almost 12 years ago
- File deleted (
332_3_uppsrc.diff)
#13 Updated by Sender Ghost almost 12 years ago
- File 332_3_uppsrc.diff added
Fixed some typos for third patch.
#14 Updated by Sender Ghost almost 12 years ago
- File deleted (
332_3_uppsrc.diff)
#15 Updated by Sender Ghost almost 12 years ago
- File 332_3_uppsrc.diff added
#16 Updated by Sender Ghost almost 12 years ago
With third patch the menu closes without the "tail" of previous menu (this is correct).
But after testing with MenuBarExecute application, I found, that destructors of MenuBar still accumulate, which causes the application to terminate for some number of executed menu in loop (in my case it was for n >= 985). This is also true for current (r5394) approach, but with many levels.
#17 Updated by Miroslav Fidler almost 12 years ago
- Status changed from Patch ready to Rejected
Sure, there is no solution to this problem...