Bug 38350 - [MPE] [KeyBindings] KeyBindingService is not virtualized in multi page editors
Summary: [MPE] [KeyBindings] KeyBindingService is not virtualized in multi page editors
Status: CLOSED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 2.1   Edit
Hardware: PC Windows 2000
: P2 major (vote)
Target Milestone: 3.0   Edit
Assignee: Nick Edgar CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-06-02 18:33 EDT by Brad Blancett CLA
Modified: 2005-03-01 15:24 EST (History)
7 users (show)

See Also:


Attachments
Source jar: virtualized key binding service (3.24 KB, application/x-zip-compressed)
2003-06-12 16:34 EDT, Nick Edgar CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brad Blancett CLA 2003-06-02 18:33:12 EDT
For a MultiPageEditorPart we are stuck with one KeyBindingService. We need to be
able to create our own KeyBindingService for each editor site. Currently, there 
is an IKeyBindingService interface. We tried to create our own kbservices using 
the interface but it failed because of the cast down in the 
WWinKeyBindingService update(workbenchPart):

void update(IWorkbenchPart workbenchPart) {
if (workbenchPart != null) {activeKeyBindingService = (KeyBindingService)
workbenchPart.getSite().getKeyBindingService();	}

KeyBindingService is an internal/final class so we can not subclass it. It 
would be ideal if it wasnt. This would solve our problem, but if not posiable 
we need a fix for the cast down in the update() method. We also need a hook 
into getAction() of the KeyBindingService so we can delegate between 
KeyBindingServices.
Comment 1 Nobody - feel free to take it CLA 2003-06-04 11:52:46 EDT
I'm not 100% sure this is what you're looking to do, but here is a potential 
solution. In my multi-page editor, I have found that an effective workaround 
to this problem is to define separate scopes and then subclass pageChange() to 
set the different scopes. After you do that you can send an event which seems 
to convince eclipse to update the scope properly so key bindings start flowing.
The code looks something like:

	protected void pageChange(int newPageIndex) {
		super.pageChange(newPageIndex);
		currentPage = newPageIndex;
		String newScope[] = new String[1];

		if (newPageIndex == 0) {
			newScope[0] = "com.ibm.xxx";
			getSite().getKeyBindingService().setScopes(newScope);
		} else if (newPageIndex == 1) {
			newScope[0] = "com.ibm.yyy";
			getSite().getKeyBindingService().setScopes(newScope);
		}

		//	This somehow triggers the refreshing of keyboard 
accelerators.
		Event e = new Event();
		e.type = SWT.Activate;
		getEditorSite().getShell().notifyListeners(SWT.Activate, e);

	}

Does that help?
Comment 2 Brad Blancett CLA 2003-06-04 18:57:38 EDT
A Scope will not work if the one of the editors replaces a global action. In 
our case, one of our editors extends the 
org.eclipse.ui.editors.text.TextEditor. In its abstract class
(org.eclipse.ui.texteditor.AbstractTextEditor) it implements global actions 
(cut,copy,paste...) and adds them to the KeyBindingService. Now these actions 
are part of the "org.eclipse.ui.globalScope", the base scope. 
Comment 3 Nick Edgar CLA 2003-06-12 14:45:50 EDT
Subclassing MultiPageEditorSite and providing a "virtualized" implementation of 
getKeyBindingService() should work.
This implementation can directly implement IKeyBindingService, and would do the 
following:
- record the actions registered by the nested editor
- when the nested editor is activated (its tab is selected), register all 
recorded actions with the outer editor's key binding service
- while the editor is active, pass on all register/unregister requested to the 
outer service, in addition to recording them
- when the nested editor is deactivated (and before activating any other nested 
editor), unregister all recorded actions from the outer editor's key binding 
service
- do similarly for the scope settings

This nested IKeyBindingService should never get passed to WWinKeyBindingService 
update.  It is reasonable for WWinKeyBindingService to assume that the 
implementation if IKeyBindingService for top-level editors is a 
KeyBindingService.  KeyBindingService should not have to be exposed, or made 
non-final.

This was the approach recommended in bug 27539 for PDE, however PDE chose 
instead to work around the problem by removing the global actions from the 
keybinding service.  More details in bug 27539.

The following code is from PDESourcePage:

	private void unregisterGlobalActions() {
		// A workaround for bug 27539
		// Unregistering important actions from
		// the key binding service allows
		// global actions to handle accelerators 
		// properly
		IKeyBindingService service = getEditor().getSite
().getKeyBindingService();
		service.unregisterAction(getAction
(ITextEditorActionConstants.DELETE));
		service.unregisterAction(getAction
(ITextEditorActionConstants.UNDO));
		service.unregisterAction(getAction
(ITextEditorActionConstants.REDO));
		service.unregisterAction(getAction
(ITextEditorActionConstants.CUT));
		service.unregisterAction(getAction
(ITextEditorActionConstants.COPY));
		service.unregisterAction(getAction
(ITextEditorActionConstants.PASTE));
		service.unregisterAction(getAction
(ITextEditorActionConstants.SELECT_ALL));
		service.unregisterAction(getAction
(ITextEditorActionConstants.FIND));
		service.unregisterAction(getAction
(ITextEditorActionConstants.BOOKMARK));
	}
Comment 4 Nick Edgar CLA 2003-06-12 16:34:57 EDT
Created attachment 5177 [details]
Source jar: virtualized key binding service

The attached jar contains source illustrating a virtualized key binding
service.
Change your code to extend MultiPageEditorPart2 instead of MultiPageEditorPart.


Please let me know if this works for you.
Comment 5 Nick Edgar CLA 2003-06-18 00:47:07 EDT
Might be able to generalize this pattern by having nestable services implement 
something like:

interface INestableService {
  INestedService newNestedInstance();
}

interface INestedService {
  void activate();
  void deactivate();
}

In this way, clients would not need to be exposed to concrete implementation 
classes like MultiPageKeyBindingService (SubActionBars would be another good 
candidate).  Services themselves would provide the ability to be nested.  For 
example, MultiPageEditorSite2 would obtain the nested key binding service 
using:
keyBindingService = ((INestableService) multiPageEditor.getSite
().getKeyBindingService()).newNestedInstance();

This could then be further generalized for arbitrary services.  If 
IWorkbenchSite extended IAdaptable, services could be obtained using getAdapter
(ISomeService.class).  The implementation of getAdapter on nested sites would 
then be something like:
Object getAdapter(Class adapter) {
  Object outerService = getOuterSite().getAdapter(adapter);
  if (outerService instanceof INestableService) {
    return ((INestableService) outerService).newNestedInstance();
  }
  else {
    return null;
  }
}

Actually, the site would have to cache the result to avoid repeatedly creating 
the nested service each time it was requested.
Comment 6 Nick Edgar CLA 2003-10-03 15:07:03 EDT
Downgrading severity and priority since the attached code allows a workaround.
We are still aware of the problem of nesting services, and are considering how 
best to solve it in general.
Comment 7 Douglas Pollock CLA 2005-02-21 11:09:47 EST
I believe this problem has already been fixed.  KeyBindingService implements
INestableKeyBindingService.  This was done specifically to address
MultiPageEditorParts.
Comment 8 Nick Edgar CLA 2005-02-21 13:44:35 EST
Yes, this was fixed in 3.0.
Comment 9 Brad Blancett CLA 2005-03-01 15:24:06 EST
cleaning house verifed