Hello Ok. But how the first approach (extending SelectionEvent and modifying EVENT_ORDER) can have an unexpected side effects? And this problem not only in RadioGroupFieldEditor. This can be reproduced in any other place where using radios with selection listeners.
I am sure the approach with tagging events on the client-side is the best approach. But I still think that I can use the first suggested approach as a temporary solution. Is there any other problems with this approach? Thank you, Igor -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Rudiger Herrmann Sent: Friday, April 17, 2009 10:00 PM To: RAP project development-related communication Subject: Re: [rap-dev] Radio buttons and selection events Hi Igor, hacking the accept method would break its contract certainly have side effects as it is used in various places. The solution which I would prefer would be to tag the events on the client-side with a sequential number. Then they could be processed on the server-side in the same order as they ocured. This of course is a larger task. To solve the problem in the meanwhile, I would rather change the RadioGroupFieldEditor. Thanks, Rüdiger mail.apptech.nichost.ru wrote: > Hello > > I found some problem with radio buttons. > > Look, for example, at org.eclipse.jface.preference.RadioGroupFieldEditor. > > Find line 267. You will see, that some selection event listener is > added to all of the radios in group. How it should work? The user selects the radio. > The old selected radio becomes an unselected (first selection event). > The new radio becames selected (second selection event). As you can > see in > RadioGroupFieldEditor#267 the order of this two events has a big > matter (unselect - the first, select - the second). It is working fine > on RCP > > But. > > It works wrong on RAP. The problem is in > org.eclipse.swt.internal.widgets.WidgetTreeVisitor#accept method. > As you can see the WidgetTreeVisitor#accept method will be called for > each item in the collection of siblings in that order in wich they was > added there. So if you have 4 radios and the last one is selected and > you try to select the third (or second or first) one the selected > event will process before the unselected event. > > It is incorrect. > > There are few ways to deal with this problem. The first and second one > is just for fun. The third one, I think, is correct. > > 1) The first solution is to create DeselectionEvent (in > org.eclipse.swt.events package, or in some other package to mark it as > internal): > > public class DeselectionEvent extends SelectionEvent { > > public DeselectionEvent( Widget widget, Widget item, int id, > Rectangle bounds, String text, boolean doit, int detail ) { > super( widget, item, id, bounds, text, doit, detail ); > } > > public DeselectionEvent( Widget widget, Widget item, int id ) { > super( widget, item, id ); > } > > public DeselectionEvent( Event e ) { > super( e ); > } > > } > > You also have to change a org.eclipse.swt.events.TypedEvent#EVENT_ORDER: > > private static final Class[] EVENT_ORDER = { > ControlEvent.class, > ActivateEvent.class, > ShowEvent.class, > DisposeEvent.class, > SetDataEvent.class, > MouseEvent.class, > VerifyEvent.class, > ModifyEvent.class, > TreeEvent.class, > CTabFolderEvent.class, > ExpandEvent.class, > FocusEvent.class, > DeselectionEvent.class, // added > SelectionEvent.class, > LocationEvent.class, > ShellEvent.class, > MenuEvent.class, > KeyEvent.class > }; > > And finally change the > org.eclipse.swt.internal.widgets.buttonkit.RadioButtonDelegateLCA. > Change readData method: to: > > void readData( final Button button ) { > // [if] The selection event is based on the request "selection" parameter > // and not on the selection event, because it is not possible to fire the > // same event (Id) from javascript for two widgets (selected and > unselected > // radio button) at the same time. > if( ButtonLCAUtil.readSelection( button ) ) { > processSelectionEvent( button, button.getSelection() ); > } > ControlLCAUtil.processMouseEvents( button ); > ControlLCAUtil.processKeyEvents( button ); } > > And processSelectionEvent method: > > private static void processSelectionEvent( final Button button, > boolean selected ) { > if( SelectionEvent.hasListener( button ) ) { > Rectangle bounds = WidgetLCAUtil.readBounds( button, > button.getBounds() ); > int type = SelectionEvent.WIDGET_SELECTED; > > SelectionEvent event; > if( selected ) { > event = new SelectionEvent( button, null, type, bounds, null, > true, SWT.NONE ); > } else { > event = new DeselectionEvent( button, null, type, bounds, null, > true, SWT.NONE ); > } > event.processEvent(); > } > } > > Alternative you can change the logic of > TypedEvent#getScheduledEvents() and add a new variable in > SelectionEvent that will tell to that method if the component is > selected. But it is the same approach. There is only one problem in > such approach. The developers will see the DeselectionEvent instead of > SelectionEvent in their applications. But it is not a big problem. I use this approach my application. > > 2) This bug can be fixed with only JS (but it will be necessary to use > two requests instead of one: one to make a unselect event and the > other to make a select event) > > 3) It is possible to make all changes in WidgetTreeVisitor#accept method: > > public static void accept( final Widget root, final WidgetTreeVisitor > visitor ) { > if( root instanceof Group ) { > Composite composite = ( Composite )root; > if( visitor.visit( composite ) ) { > handleMenus( composite, visitor ); > handleItems( root, visitor ); > Control[] children = composite.getChildren(); > Set accepted = new HashSet(); > for (int i=0; i<children.length; i++) { > if (children[i] instanceof Button && > ((Button)children[i]).getSelection()) { > accept( children[ i ], visitor ); > accepted.add( children[ i ] ); > } > } > for( int i = 0; i < children.length; i++ ) { > if (!accepted.contains( children[ i ] )) { > accept( children[ i ], visitor ); > } > } > } > } else if( root instanceof Composite) { > Composite composite = ( Composite )root; > if( visitor.visit( composite ) ) { > handleMenus( composite, visitor ); > handleItems( root, visitor ); > Control[] children = composite.getChildren(); > for( int i = 0; i < children.length; i++ ) { > accept( children[ i ], visitor ); > } > } > } else if( ItemHolder.isItemHolder( root ) ) { > if( visitor.visit( root ) ) { > handleItems( root, visitor ); > } > } else { > visitor.visit( root ); > } > } > > But it looks like a temporary solution. There is only one advantage in > this > approach: the developers will see the SelectionEvent as they can see > it on RCP in selection listeners. > > Thank you, Igor > > > _______________________________________________ > rap-dev mailing list > [email protected] > https://dev.eclipse.org/mailman/listinfo/rap-dev _______________________________________________ rap-dev mailing list [email protected] https://dev.eclipse.org/mailman/listinfo/rap-dev _______________________________________________ rap-dev mailing list [email protected] https://dev.eclipse.org/mailman/listinfo/rap-dev
