Bug #332

CtrlLib: MenuBar::Execute changes

Added by Sender Ghost over 11 years ago. Updated over 11 years ago.

Status:RejectedStart date:09/27/2012
Priority:HighDue date:
Assignee:Miroslav Fidler% Done:

100%

Category:CtrlLibSpent 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.

332_2_uppsrc.diff Magnifier - The diff file to apply for uppsrc directory (second patch) (780 Bytes) Sender Ghost, 09/30/2012 08:03 AM

MenuBarExecute.zip - MenuBar::Execute test (1.25 KB) Sender Ghost, 10/04/2012 09:12 AM

332_3_uppsrc.diff Magnifier - The diff file to apply for uppsrc directory (third patch) (1.33 KB) Sender Ghost, 10/04/2012 09:48 AM

History

#1 Updated by Miroslav Fidler over 11 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 over 11 years ago

  • Status changed from Ready for QA to Approved

Yes, now it works. Thanks.

#3 Updated by Sender Ghost over 11 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 over 11 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 over 11 years ago

  • File deleted (332_uppsrc.diff)

#6 Updated by Sender Ghost over 11 years ago

I have added the second patch with possible solution for the (new) issue.

#7 Updated by Sender Ghost over 11 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 over 11 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 over 11 years ago

  • Assignee changed from Miroslav Fidler to Sender Ghost

#10 Updated by Sender Ghost over 11 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 over 11 years ago

Also, there is some kind of simple application to test MenuBar::Execute (for single menu).

#12 Updated by Sender Ghost over 11 years ago

  • File deleted (332_3_uppsrc.diff)

#13 Updated by Sender Ghost over 11 years ago

  • File 332_3_uppsrc.diff added

Fixed some typos for third patch.

#14 Updated by Sender Ghost over 11 years ago

  • File deleted (332_3_uppsrc.diff)

#15 Updated by Sender Ghost over 11 years ago

#16 Updated by Sender Ghost over 11 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 over 11 years ago

  • Status changed from Patch ready to Rejected

Sure, there is no solution to this problem...

Also available in: Atom PDF