Community
Participate
Working Groups
Attaching contributed patches to support a setMenu method on ToolItem.
Created attachment 71745 [details] SWT Patch
Created attachment 71746 [details] SWT Tools patch
Created attachment 71747 [details] Snippets patch
Created attachment 71748 [details] JFace patch Optional patch that allows JFace to make use of the new functionality
Created attachment 71749 [details] PDF File describing implementation
Cool :)
OK, so I take this to mean that the answer to my question is "Yes, this enhancement will be incorporated into SWT." However, as I said in my original message (http://dev.eclipse.org/mhonarc/lists/platform-swt-dev/msg05459.html), this is still work in progress. I have discovered additional areas of jface that can take advantage of the improved drop-down behavior, such as buttons that do not have a DROPDOWN style but yet serve only to display drop-down menu (e.g. the buttons at the top-right corner of many views s.a. Package Explorer). To accommodate this additional case the design will have to be changed, possibly by introducing an additional SWT style. I will post updated versions of my patches later as as I find the time to work on this. However, before I delve into this any further, I am really anxious to get feedback on the design of the code. For example, is the use of sendEvent() as opposed to postEvent() OK? Are there any issues related to that or something else I could be missing. The whole motivation behind using postEvent as opposed to sendEvent in SWT escapes me, which is why I ask. Seeking feedback was the very goal of my message on the swt-dev mailing list but since Bogdan already created the bugzilla report, I am writing here in the hope that I can get more input here. So far the feedback seems positive, even if only on the user side.
Clarifications on the summary: 1. This functionality is independent of the existing get/setMenu() support provided by the Control class. A ToolItem can potentially have both a popup menu (activated on a right-click and set via setMenu) and a drop-down menu (activated on click of the button's arrow and set via setDropDownMenu) at the same time. 2. For completeness, this enhancement should (if possible) include support for dropp-down menus on CoolItem in addition to ToolItem.
while only win32/win32 contains code to implement native behavior, the SWT patch includes updated versions of the ToolItem class for all platforms. sanity check has been done on win32/win32, win32/wpf, linux/gtk and linux/motif. should OS be set to All?
*** Bug 48914 has been marked as a duplicate of this bug. ***
OS should be set to "All". We have been considering this API for a while. There is a reason that the event is posted rather than sent was a pixel corruption bug (bug 123885 captures the "arrow not down" issue and talks about the pixel corruption) but this may be gone now. Also, when a popup menu is displayed, it does not block execution until the menu returns. Consider this code: System.out.print ("HI"); menu.setVisible(true); System.out.println ("THERE"); On all platforms, it will print "HI THERE" and then the menu will pop up.
*** Bug 47065 has been marked as a duplicate of this bug. ***
*** Bug 5088 has been marked as a duplicate of this bug. ***
*** Bug 6425 has been marked as a duplicate of this bug. ***
Thank you Steve. The bug you linked to does provide a lot of insight rearding sendEvent/postEvent and the "asynchronous" nature of Menu.setVisible(). However, I was not able to figure out what you mean by the "pixel corruption bug". In the other bug report's comment you just mention this: "Once upon a time, there was a cheese bug where the pushed in tool item didn't redraw. We worked around that in another bug report. I'm afraid of changing the post to a send though." Anyway, here are my comments and a proposal: * By introducing the new API I am able to avoid the setVisible issue because I can call in directly to the _setVisible method and block on the TrackPopupMenu call as is needed to achieve the native behavior on Windows. * I needed to do a sendEvent of the SWT.Selection event in order to allow the user to create and assign the drop-down on demand (i.e. when the button is pressed). This is critical since Eclipse, for example, generates its dropdown menus on demand in an SWT.Selection handler. I have the following proposal, that I would like to hear your opinion on: * Introduce a new style called SWT.DROP_DOWN_MENU (or another more appropriate name) which works on ToolItem and CoolItem independently of the existing SWT.DROP_DOWN style, and indicates that the user would like to use the new setDropDownMenu functionality. * For SWT.Selection + SWT.Arrow events do a sendEvent *only* if the new flag has been set; postEvent otherwise. This preserves the behavior completely for existing code. * The style DROP_DROP_DOWN_MENU will be set in the event details for those sent events to indicate that this event is a notification that the drop-down menu is about to be shown. For regular buttons (those without SWT.DROP_DOWN arrows) this event will be sent before the MouseDown event and a regular SWT.Selection event will not be sent on mouse-up for the button unless the user cancels the drop-down by setting event.doit to false. This, I believe will result in a most flexible API, which gives the user enough power to control drop-down menus as one likes, while preserving full compatibility with existing code. If this change is deemed appropriate and accepted, I can implement it sometime in the next couple of weeks and add updated patches here. (In reply to comment #11)
Right, but we have decided in the past that we need the postEvent() and that doing sendEvent() caused pixel corruption ("cheese", "garbage on the screen"). This may be gone now. It's too bad the bug report didn't include a screen shot. So before we change postEvent() to sendEvent() we need to be sure that the pixel corruption doesn't come back (on all platforms).
I don't like SWT.DROP_DOWN_MENU because it adds another style for a concept that we already have. The style bit will get confused with SWT.DROP_DOWN. I'd rather work on changing the postEvent() to a sendEvent(). I think that calling _setVisible() is OK in this case, since the menu request for the tool bar comes from the event loop and we are showing the menu, the programmer can't tell that we are doing a modal loop in the tool item rather than in readAndDispatch().
(In reply to comment #17) OK. I read through the comments on bug 47065. Here is another idea: Add a DropDownMenuDetectEvent event which is sent before the drop-down menu is displayed; leave everything else intact, i.e. dispatch SWT.Selection via postEvent. And some thoughts on this: 1) The SWT.Selection event might have to be cancelled in the case when SWT displays a drop-down menu to avoid incompatibilities/unexpected behavior. 2) I just realized that ToolItem does not support the setMenu() API for popup menus. In this case, the dropDownMenu property might be renamed to just Menu and the existing MenuDetectEvent reused. However, imo it is better to keep the name DropDownMenu, because the behavior and function of popup and drop-down menus differ in many respects. Furthermore, I looked over the API and it does not seem like a lot of effort to add a DropDownMenuDetectEvent + DropDownMenuDetectListener and all of its other supporting classes. Finally, I am willing to commit the time to do it. P.S. As a reminder: the whole point of sendEvent vs postEvent is to allow Eclipse to easily take advantage of the fix, because Eclipse tends to generate menus on demand.
The best API's build on concepts and names that you already know. This makes them smaller both in conceptual space and in memory. New names and concepts fight their way in. If you can add functionality without new concepts, then you are doing your clients a big service. Since it is not possible to cancel a drop down menu from a menu bar, I don't see why we need to offer that capability for tool bars. If you both add a menu using ToolItem.setMenu() and show one in SWT.Selection, then that's your problem. Programmers exect to see SWT.MenuDetect when the user asks for a menu, no matter where or how. For example, Shift+F10 requests a context menu from a control. Therefore, we should keep this name for when users request a menu from a ToolItem.
If we show the menu, we could consider using TrackPopupMenuEx() to fix bug 156734 using TPMPARAMS and an exclusion rectangle.
OK, I will rename ToolItem.setDropDownMenu to setMenu, add code to send MenuDetect and add support for setMenu on SWT.PUSH ToolItems (in the patches attached here currently setDropDownMenu only works on SWT.ARROW ToolItems). I will revert the SWT.Selection dispatch code to use postEvent. I should have something up here in 1-2 weeks. The current patches already use TrackPopupMenuEx for displaying the ToolItem's menus.
Also, we would never define a helper class like ToolItemHelper for 10 lines of code (or whatever) to attempt to share them between plaforms. For one thing, we minimize the number of classes in SWT so it loads faster and uses less memory. Also, experience has shown that when we try to share little bits like this, we end up having to edit the shared code over time to include non-share concepts like operating system structures. I saw you changed all the platforms. You didn't test them all right? BTW, in case I didn't say so already, THANKS!! I've been meaning to get to this for a while. Forgot that newer versions of GTK support the drop down tool menu item concept natively (another reason to avoid the helper class) so just submit your patches for Windows.
*** Bug 44706 has been marked as a duplicate of this bug. ***
I have a pretty good idea what is needed for Windows. Kevin, can you please investigate the other platforms. NOTE: There are unresolved issues wrt. when the menu should be disposed. The rule for Control.setMenu() and MenuItem.setMenu() is that is will be disposed when the "owner" is disposed because there isn't a parent/child relationship. Carolyn, if we change this, we will have to generalize and update Rule #2 of the "rules for calling dispose()" documentation where ever it occurs. ASIDE: TrayItem.setMenu() although unrelated, is another area where built-in API to support menus might be a possibility.
Created attachment 72862 [details] SWT Patch v2 This is the new implementation of ToolBar.setMenu. Here are the noteworthy details: * The patch was created against the latest versions of files from HEAD as of 7/1/07. * Only a win32 implementation is included in this patch. * Both SWT.DROPDOWN and SWT.PUSH items support setMenu and MenuDetect listeners. * The support for menus on PUSH items allows items such as the New Perspective Contribution button and the View Pulldown Menu button to exhibit fully compliant platform behavior. I have an updated jface patch and a new workbench.ui patch that tackes advantage of the new PUSH item menu capability but am not uploading those here now. I figure I can post them to the appropriate projects if this patch makes it to the SWT source tree. * The menu can be set in a MenuDetectListener, which is invoked right before the menu is to be displayed on the screen. * Menus for both DROPDOWN and PUSH items can be triggered by using the Up and Down keys on the hot item of a FLAT toolbar that has focus. * A DROPDOWN ToolItem that has a menu installed through setMenu does not receive ARROW Selection events. * A PUSH ToolItem that has a menu installed through setMenu does not receive Selection events. If a MenuDetect event is cancelled with event.doit=false, however, a selection event will be sent. * When a menu is triggered by using the mouse (either on click or double-click) some mouse events are eaten (i.e. mouselisteners attached to the toolbar will not receive them). These mouse events are the mouseup event after activating the menu and the mousedown event that closes the menu (if the menu was closed by click on the toolbar again). Here are some issues I have identified so far and have been unable to resolve: * For PUSH items a mousedown/mousedbllcick event that triggers the menu is eaten as well. DROPDOWN items deliver this event right before displaying the menu. Look at the Snippet67x file in the updated snippets patch for a demo that showcases the mouse event eating patterns. * When discarding the menu of a PUSH item by pressing and holding the left mouse button over the window's titlebar the button that had the menu will remain pressed for about 1 second. I have actually seen this issue with Swing menus as well - on Java 6, when discarding a swing menu by pressing and holding the mouse over the window's titlebar, the menu only disappears after about 1 second.
Created attachment 72863 [details] Snippets patch v2
Created attachment 72930 [details] SWT Patch v2.1 All the issues that I mentioned about v2 are resolved. Support is there for CoolItem.setMenu, so that chevron buttons can also follow proper platform behavior. New code to validate the menu passed to setMenu, copy-pasted from Control.setMenu New code to dispose the menu in releaseWidget, copy-pasted from Control.setMenu Sorted the members in updated files Updated javadoc for new menthods to make it a bit more relevant (it is not complete though) In my testing with modified snippets and modified eclipse plugins to take advantage of the new funcitonality, everything seems to work perfectly now. I rely heavily on win32 apis to make menus work for push items and coolitems, so I am sure there will be some changes required to make this work correctly on CE. Also the CE dll build might be broken by my addition of the TrackPopupMenuEx function.
Created attachment 72931 [details] Snippets patch v2.1 Updated Snippet140 to take demonstrate a chevron button with proper platform behavior.
(In reply to comment #25) > * A DROPDOWN ToolItem that has a menu installed through setMenu does not receive > ARROW Selection events. > * A PUSH ToolItem that has a menu installed through setMenu does not receive > Selection events. If a MenuDetect event is cancelled with event.doit=false, > however, a selection event will be sent. Update: a DROPDOWN ToolItem whose MenuDetect event is cancelled will also post an ARROW selection event, whether it has a menu set or not.
Hi, It has been a while since I uploaded a complete, working implementation of ToolITem/CoolItem.setMenu on the win32 platform here. Feel free to check it out and provide feedback. In the next several weeks, if I find the time I will try to implement the same support on several additional platforms.
Does anyone plan to ever attend to this bug?
Created attachment 90591 [details] unified patch against head as of 2008-02-24 here is an updated patch against the latest code in HEAD. this includes win32.win32.x86 and x86_64 platforms
Created attachment 90592 [details] jface and ui.workbench patch to take advantage of setMenu here is an updated patch against the latest code in HEAD.
Created attachment 91606 [details] swt setmenu unified patch for 3.4 head
Created attachment 91607 [details] swt setmenu wpf patch wpf does not implement CoolItem chevron button API yet, so this patch only adds the public setMenu API to CoolItem but does not implement it. ToolItem setMenu works as it would if menus were manually implemented ala Snippet67
Now that M6 is out, Eclipse is in API freeze mode and in all likelihood the setMenu api will not make it in the 3.4 release this June. I have noticed that there are a few people from third-party Eclipse tool vendors on the CC list here. If you are interested in seeing the setMenu API in Eclipse (maybe in 4.0) I encourage you to vote for this bug. -Nikolay P.S. I was informed that this bug was considered for inclusion in 3.4 but I was never notified of a decision.
Bug 5088 (marked as dup of this bug) is rather annoying on Linux. Since the menu does not show up on mouseDown, I get no feedback whether I've hit the target area for the dropdown menu or whether releasing the mouse will execute an action. There's also no visual separator to help me hit the drop-down area. Either this bug should not be an enhancement, or bug 5088 is not a dup if this.
I can update the bug patches for windows and now also implement and test on the all major platforms: win x86 gdi win x86_64 gdi win x86 wpf win x86_64 wpf mac carbon mac cocoa linux x86 gtk linux x86_64 gtk solaris x86 gtk solaris sparc gtk It will be a lot of work but I can do it over the next few months... if someone can assure me that there is a good prospect of the fix making it in the next release of Eclipse. Thanks (In reply to comment #37) > Bug 5088 (marked as dup of this bug) is rather annoying on Linux. > > Since the menu does not show up on mouseDown, I get no feedback whether I've > hit the target area for the dropdown menu or whether releasing the mouse will > execute an action. There's also no visual separator to help me hit the > drop-down area. > > Either this bug should not be an enhancement, or bug 5088 is not a dup if this. >
add to that motif/linux/x86 and that should cover all window systems currently supported by eclipse: gdi wpf gtk2 motif cocoa carbon (In reply to comment #38) > I can update the bug patches for windows and now also implement and test on the > all major platforms: > > win x86 gdi > win x86_64 gdi > win x86 wpf > win x86_64 wpf > mac carbon > mac cocoa > linux x86 gtk > linux x86_64 gtk > solaris x86 gtk > solaris sparc gtk > > It will be a lot of work but I can do it over the next few months... if someone > can assure me that there is a good prospect of the fix making it in the next > release of Eclipse. > > > Thanks > > > (In reply to comment #37) > > Bug 5088 (marked as dup of this bug) is rather annoying on Linux. > > > > Since the menu does not show up on mouseDown, I get no feedback whether I've > > hit the target area for the dropdown menu or whether releasing the mouse will > > execute an action. There's also no visual separator to help me hit the > > drop-down area. > > > > Either this bug should not be an enhancement, or bug 5088 is not a dup if this. > > >
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.
