Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 337384

Summary: remove printout of 'no binding table...' to std err
Product: [Eclipse Project] Platform Reporter: Brian de Alwis <bsd>
Component: UIAssignee: Brian de Alwis <bsd>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: beth, henrik.lindberg, Michael_Rennie, pwebster, torkildr
Version: 4.2   
Target Milestone: 4.2.2   
Hardware: All   
OS: All   
Whiteboard:

Description Brian de Alwis CLA 2011-02-16 19:01:08 EST 
BindingServiceImpl #activateBinding() ignores bindings when the specified binding table does not exist.  It should create the table and hook it into the model.

	public void activateBinding(Binding binding) {
		String contextId = binding.getContextId();
		BindingTable table = manager.getTable(contextId);
		if (table == null) {
			System.err.println("No binding table for " + contextId); //$NON-NLS-1$
			return;
		}
		table.addBinding(binding);
	}
Comment 1 Paul Webster CLA 2011-02-17 13:57:30 EST 
with the processing addon adding the MBindingContext and potentially the MBindingTable should fill in the running service binding table system.

But why is the binding context not defined?  Is this an e4 problem, or a compat/plugin.xml problem?  Or are we running functionality out of order?

PW
Comment 2 Brian de Alwis CLA 2011-02-17 14:36:56 EST 
(In reply to comment #1)
> But why is the binding context not defined?  Is this an e4 problem, or a
> compat/plugin.xml problem?  Or are we running functionality out of order?

When running, I see:

No binding table for org.eclipse.ui.contexts.dialog
No binding table for org.eclipse.ui.contexts.dialog

Those are the System.err.println in #activateBinding and #deactiveBinding().  There are two outputs as I first call #deactivateBinding() followed by an #activateBinding() as #activateBinding() doesn't check if the binding already exists.

This processing is done through an addon too, but the addon is itself installed through a fragment processor.  The processor specifies beforefragment="false", and the addon is added to the end of the list, so it *should* run after binding addon.  And indeed I see the order is: BindingServiceAddon, BindingProcessingAddon, CocoaUIHandler.

The code in question in the CocoaUIHandler does the following:

	private static final String COMMAND_ID_CLOSE_DIALOG = "org.eclipse.ui.cocoa.closeDialog"; //$NON-NLS-1$
	private static final String CLOSE_DIALOG_KEYSEQUENCE = "M1+W"; //$NON-NLS-1$
	private static final String DEFAULT_ACCELERATOR_CONFIGURATION = "org.eclipse.ui.defaultAcceleratorConfiguration"; //$NON-NLS-1$

		TriggerSequence sequence = bindingService
				.createSequence(CLOSE_DIALOG_KEYSEQUENCE);
		ParameterizedCommand cmd = commandService.createCommand(
				COMMAND_ID_CLOSE_DIALOG, null);
		Binding binding = bindingService.createBinding(sequence, cmd,
				DEFAULT_ACCELERATOR_CONFIGURATION,
				EBindingService.DIALOG_CONTEXT_ID);
		bindingService.deactivateBinding(binding);
		bindingService.activateBinding(binding);
Comment 3 Beth Tibbitts CLA 2012-06-04 19:30:27 EDT 
I also get this on Mac OSX 10.6.8 (Snow Leopard)
on Juno, most recently RC2, on eclipse SDK ("eclipse classic"), epp parallel package, and others.

Does this mean there is something wrong? Will be hard to explain to users.
Comment 4 Paul Webster CLA 2012-06-05 08:17:07 EDT 
Brian, I thought you updated the code to add the binding table on mac?

PW
Comment 5 Brian de Alwis CLA 2012-06-05 08:40:50 EDT 
This is a harmless warning message that can be safely ignored; we could comment it out.

Paul: I didn't change activateBinding to create the table — in looking at bug 353628, I determined the only consequence was that the close-dialog binding was effectively ignored, and didn't seem a high-priority must-fix compared to the other issues I was investigating at the time.

Marking this for 4.2.1.
Comment 6 Paul Webster CLA 2012-06-05 08:56:10 EDT 
I don't know that I want activateBinding to create tables randomly.

Our Eclipse4 system should be "update model correctly" -> bindings update correctly.

PW
Comment 7 Brian de Alwis CLA 2012-06-05 09:01:34 EDT 
Ok, makes sense.  closing as WONTFIX.
Comment 8 Beth Tibbitts CLA 2012-06-05 09:48:05 EDT 
WONTFIX ..  But ... will the error message be commented out?
Comment 9 Henrik Lindberg CLA 2012-08-21 11:23:31 EDT 
Reopen and rename to "remove printout of 'no binding table...' to std err" ?
(please).
Comment 10 Paul Webster CLA 2012-08-21 12:01:53 EDT 
we should either log it or remove it completely.

PW
Comment 11 Brian de Alwis CLA 2012-08-21 12:17:22 EDT 
The logging statement's been removed for 4.2.1.  This bug was about activateBindings and got (inadvertently) hijacked.