Bug 335985 - Adapt the KeyController model to the e4 model
Preliminary approach using the runtime system only
diff --git a/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/EBindingService.java b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/EBindingService.java
index 6462efd..308884b 100644
--- a/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/EBindingService.java
+++ b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/EBindingService.java
@@ -19,8 +19,10 @@
 
 	public static final String DIALOG_CONTEXT_ID = "org.eclipse.ui.contexts.dialog"; //$NON-NLS-1$
 
+	// TODO perhaps use a map of attributes for things
+	// that aren't important to the model
 	Binding createBinding(TriggerSequence sequence, ParameterizedCommand command, String schemeId,
-			String contextId);
+			String contextId, String locale, String platform, int bindingType);
 
 	void activateBinding(Binding binding);
 
diff --git a/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingCopies.java b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingCopies.java
new file mode 100644
index 0000000..0d38038
--- /dev/null
+++ b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingCopies.java
@@ -0,0 +1,76 @@
+/*******************************************************************************
+ * Copyright (c) 2011 IBM Corporation and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ ******************************************************************************/
+
+package org.eclipse.e4.ui.bindings.internal;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import org.eclipse.core.commands.ParameterizedCommand;
+import org.eclipse.jface.bindings.Binding;
+import org.eclipse.jface.bindings.TriggerSequence;
+
+public class BindingCopies {
+
+	private static Collection<Binding> sysBindings;
+	private static Collection<Binding> inactiveSystemBindings;
+	private static Collection<Binding> userBindings;
+
+	public static void init() {
+		inactiveSystemBindings = new ArrayList<Binding>();
+		userBindings = new ArrayList<Binding>();
+	}
+
+	public static void setDefaultSysBindings(Collection<Binding> newSysBindings) {
+		sysBindings = newSysBindings;
+	}
+
+	public static Collection<Binding> getSystemBindings() {
+		return sysBindings;
+	}
+
+	public static void addInactiveSysBinding(Binding binding) {
+		inactiveSystemBindings.add(binding);
+	}
+
+	public static Binding[] getInactiveSysBindings() {
+		return inactiveSystemBindings.toArray(new Binding[inactiveSystemBindings.size()]);
+	}
+
+	public static void removeInactiveSysBinding(Binding binding) {
+		inactiveSystemBindings.remove(binding);
+	}
+
+	public static void addUserBinding(Binding b) {
+		userBindings.add(b);
+	}
+
+	public static Binding[] getUserDefinedBindings() {
+		return userBindings.toArray(new Binding[userBindings.size()]);
+	}
+
+	public static boolean isUserBinding(TriggerSequence sequence, ParameterizedCommand command,
+			String schemeId, String contextId) {
+		boolean isUserBinding = false;
+		Binding currBinding;
+		Iterator<Binding> iter = userBindings.iterator();
+		while (iter.hasNext() && !isUserBinding) {
+			currBinding = iter.next();
+			if (currBinding.getTriggerSequence().equals(sequence)
+					&& currBinding.getParameterizedCommand().equals(command)
+					&& currBinding.getSchemeId().equals(schemeId)
+					&& currBinding.getContextId().equals(contextId)) {
+				isUserBinding = true;
+			}
+		}
+		return isUserBinding;
+	}
+}
diff --git a/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingServiceImpl.java b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingServiceImpl.java
index 183a956..6f60e53 100644
--- a/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingServiceImpl.java
+++ b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingServiceImpl.java
@@ -53,9 +53,10 @@
 	 * java.lang.String)
 	 */
 	public Binding createBinding(TriggerSequence sequence, ParameterizedCommand command,
-			String schemeId, String contextId) {
-		return new KeyBinding((KeySequence) sequence, command, schemeId, contextId, null, null,
-				null, Binding.SYSTEM);
+			String schemeId, String contextId, String locale, String platform, int bindingType) {
+
+		return new KeyBinding((KeySequence) sequence, command, schemeId, contextId, locale,
+				platform, null, bindingType);
 	}
 
 	/*
@@ -70,7 +71,6 @@
 		BindingTable table = manager.getTable(contextId);
 		if (table == null) {
 			System.err.println("No binding table for " + contextId); //$NON-NLS-1$
-			return;
 		}
 		table.addBinding(binding);
 	}
@@ -87,7 +87,6 @@
 		BindingTable table = manager.getTable(contextId);
 		if (table == null) {
 			System.err.println("No binding table for " + contextId); //$NON-NLS-1$
-			return;
 		}
 		table.removeBinding(binding);
 	}
@@ -201,4 +200,5 @@
 		}
 		contextSet = manager.createContextSet(contexts);
 	}
+
 }
diff --git a/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingTable.java b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingTable.java
index db071c1..d4491f3 100644
--- a/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingTable.java
+++ b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingTable.java
@@ -86,7 +86,6 @@
 	};
 
 	private Context tableId;
-
 	private ArrayList<Binding> bindings = new ArrayList<Binding>();
 	private Map<TriggerSequence, Binding> bindingsByTrigger = new HashMap<TriggerSequence, Binding>();
 	private Map<ParameterizedCommand, ArrayList<Binding>> bindingsByCommand = new HashMap<ParameterizedCommand, ArrayList<Binding>>();
@@ -143,7 +142,10 @@
 		bindings.remove(binding);
 		bindingsByTrigger.remove(binding.getTriggerSequence());
 		ArrayList<Binding> sequences = bindingsByCommand.get(binding.getParameterizedCommand());
-		sequences.remove(binding);
+
+		if (sequences != null) {
+			sequences.remove(binding);
+		}
 		TriggerSequence[] prefs = binding.getTriggerSequence().getPrefixes();
 		for (int i = 1; i < prefs.length; i++) {
 			ArrayList<Binding> bindings = bindingsByPrefix.get(prefs[i]);
@@ -176,4 +178,8 @@
 		return bindingsByPrefix.get(seq) != null;
 	}
 
+	public Collection<Binding> getBindings() {
+		return Collections.unmodifiableCollection(bindings);
+	}
+
 }
diff --git a/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingTableManager.java b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingTableManager.java
index 951e834..d5e3b2e 100644
--- a/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingTableManager.java
+++ b/bundles/org.eclipse.e4.ui.bindings/src/org/eclipse/e4/ui/bindings/internal/BindingTableManager.java
@@ -11,16 +11,16 @@
 
 package org.eclipse.e4.ui.bindings.internal;
 
-import org.eclipse.e4.core.contexts.IEclipseContext;
-
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.ListIterator;
 import javax.inject.Inject;
 import org.eclipse.core.commands.ParameterizedCommand;
 import org.eclipse.core.commands.contexts.Context;
+import org.eclipse.e4.core.contexts.IEclipseContext;
 import org.eclipse.jface.bindings.Binding;
 import org.eclipse.jface.bindings.TriggerSequence;
 
@@ -31,6 +31,8 @@
 	@Inject
 	private IEclipseContext eclipseContext;
 
+	HashSet<String> definedTables = new HashSet<String>();
+
 	public void addTable(BindingTable table) {
 		String contextId = table.getId();
 		if (eclipseContext.containsKey(contextId)) {
@@ -38,6 +40,7 @@
 			//			throw new IllegalArgumentException("Already contains table " + contextId); //$NON-NLS-1$
 		}
 		eclipseContext.set(contextId, table);
+		definedTables.add(contextId);
 	}
 
 	public void removeTable(BindingTable table) {
@@ -46,12 +49,26 @@
 			throw new IllegalArgumentException("Does not contains table " + contextId); //$NON-NLS-1$
 		}
 		eclipseContext.remove(contextId);
+		definedTables.remove(contextId);
 	}
 
 	public BindingTable getTable(String id) {
 		return (BindingTable) eclipseContext.get(id);
 	}
 
+	// we're just going through each binding table, and returning a
+	// flat list of bindings here
+	public Collection<Binding> getActiveBindings() {
+		ArrayList<Binding> bindings = new ArrayList<Binding>();
+		for (String id : definedTables) {
+			Object obj = eclipseContext.get(id);
+			if (obj instanceof BindingTable) {
+				bindings.addAll(((BindingTable) obj).getBindings());
+			}
+		}
+		return bindings;
+	}
+
 	public ContextSet createContextSet(Collection<Context> contexts) {
 		return new ContextSet(contexts);
 	}
diff --git a/bundles/org.eclipse.e4.ui.workbench.swt/src/org/eclipse/e4/ui/workbench/swt/util/BindingProcessingAddon.java b/bundles/org.eclipse.e4.ui.workbench.swt/src/org/eclipse/e4/ui/workbench/swt/util/BindingProcessingAddon.java
index 820ab50..5259784 100644
--- a/bundles/org.eclipse.e4.ui.workbench.swt/src/org/eclipse/e4/ui/workbench/swt/util/BindingProcessingAddon.java
+++ b/bundles/org.eclipse.e4.ui.workbench.swt/src/org/eclipse/e4/ui/workbench/swt/util/BindingProcessingAddon.java
@@ -11,6 +11,10 @@
 
 package org.eclipse.e4.ui.workbench.swt.util;
 
+import org.eclipse.e4.ui.model.application.commands.MBindingContext;
+
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -26,6 +30,7 @@
 import org.eclipse.e4.core.contexts.IEclipseContext;
 import org.eclipse.e4.core.services.events.IEventBroker;
 import org.eclipse.e4.ui.bindings.EBindingService;
+import org.eclipse.e4.ui.bindings.internal.BindingCopies;
 import org.eclipse.e4.ui.bindings.internal.BindingTable;
 import org.eclipse.e4.ui.bindings.internal.BindingTableManager;
 import org.eclipse.e4.ui.internal.workbench.Activator;
@@ -79,6 +84,7 @@
 	@PostConstruct
 	public void init() {
 		defineBindingTables();
+		cleanTables();
 		activateContexts(application);
 		registerModelListeners();
 	}
@@ -107,6 +113,71 @@
 		}
 	}
 
+	// goes through the entire active bindings list and replaces SYSTEM
+	// bindings with any USER bindings that were persisted
+	private void cleanTables() {
+		ArrayList<Binding> dirtyBindings = new ArrayList<Binding>();
+		Binding dirtyBinding;
+
+		Binding[] userBindings = BindingCopies.getUserDefinedBindings();
+		Binding curr;
+
+		// go through all USER bindings and check if there is an "equal" SYSTEM
+		// binding
+		for (int i = 0; i < userBindings.length; i++) {
+			curr = userBindings[i];
+
+			// they should all be USER bindings, but double check anyway
+			if (curr.getType() == Binding.USER) {
+				dirtyBinding = checkDirty(curr);
+
+				// if the SYSTEM binding is marked as dirty, then throw it in a
+				// list and we'll remove it later
+				if (dirtyBinding != null) {
+					dirtyBindings.add(dirtyBinding);
+				}
+			}
+		}
+
+		//System.out.println("@@@ dirty bindings -> " + dirtyBindings.size());
+
+		// go through the list of bindings that are marked as dirty (if any) and
+		// remove them from the BindingTableManager
+		for (int i = 0; i < dirtyBindings.size(); i++) {
+			dirtyBinding = dirtyBindings.get(i);
+			bindingTables.getTable(dirtyBinding.getContextId()).removeBinding(
+					dirtyBinding);
+		}
+	}
+
+	private Binding checkDirty(Binding b) {
+		Collection<Binding> activeBindings = bindingTables.getActiveBindings();
+		Iterator<Binding> iter = activeBindings.iterator();
+		Binding curr;
+		Binding dirtyBinding = null;
+
+		while (iter.hasNext() && dirtyBinding == null) {
+			curr = iter.next();
+
+			// make sure we're only comparing SYSTEM bindings so that we don't
+			// remove the wrong ones, and make sure the bindings we're comparing
+			// actually have a command
+			if (curr.getType() == Binding.SYSTEM
+					&& curr.getParameterizedCommand() != null
+					&& b.getParameterizedCommand() != null) {
+				if (curr.getContextId().equals(b.getContextId())
+						&& curr.getParameterizedCommand().equals(
+								b.getParameterizedCommand())
+						&& curr.getSchemeId().equals(b.getSchemeId())) {
+
+					// mark this binding as dirty, and it will be removed
+					dirtyBinding = curr;
+				}
+			}
+		}
+		return dirtyBinding;
+	}
+
 	private void defineBindingTables() {
 		Activator.trace(Policy.DEBUG_CMDS,
 				"Initialize binding tables from model", null); //$NON-NLS-1$
@@ -142,6 +213,7 @@
 				binding.getCommand(), binding.getParameters(),
 				binding.getKeySequence(), binding);
 		if (keyBinding != null) {
+			// if (keyBinding.getType() == Binding.USER)
 			bindingTable.addBinding(keyBinding);
 		}
 	}
@@ -168,11 +240,22 @@
 			System.err.println("Failed to handle binding: " + binding); //$NON-NLS-1$
 		} else {
 			try {
+				int bindingType = Binding.SYSTEM;
+
+				// go thru the copied list of USER defined bindings to see if
+				// this particular binding being created matches any of them
+				if (BindingCopies.isUserBinding(sequence, cmd,
+						"org.eclipse.ui.defaultAcceleratorConfiguration",
+						bindingContext.getId())) {
+					bindingType = Binding.USER;
+				}
+
+				// TODO: NEED TO CHANGE THIS!!!
 				keyBinding = bindingService
 						.createBinding(
 								sequence,
 								cmd,
-								"org.eclipse.ui.defaultAcceleratorConfiguration", bindingContext.getId()); //$NON-NLS-1$
+								"org.eclipse.ui.defaultAcceleratorConfiguration", bindingContext.getId(), null, null, bindingType); //$NON-NLS-1$
 			} catch (IllegalArgumentException e) {
 				Activator.trace(Policy.DEBUG_MENUS,
 						"failed to create: " + binding, e); //$NON-NLS-1$
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/ExtensionFactory.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/ExtensionFactory.java
index 3a56da0..0744e93 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/ExtensionFactory.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/ExtensionFactory.java
@@ -25,7 +25,8 @@
 import org.eclipse.ui.internal.dialogs.PerspectivesPreferencePage;
 import org.eclipse.ui.internal.dialogs.ViewsPreferencePage;
 import org.eclipse.ui.internal.dialogs.WorkbenchPreferencePage;
-import org.eclipse.ui.internal.keys.NoKeysPreferencePage;
+import org.eclipse.ui.internal.keys.KeysPreferencePage;
+import org.eclipse.ui.internal.keys.NewKeysPreferencePage;
 import org.eclipse.ui.internal.progress.ProgressView;
 import org.eclipse.ui.internal.themes.ColorsAndFontsPreferencePage;
 import org.eclipse.ui.internal.wizards.preferences.PreferencesExportWizard;
@@ -159,10 +160,10 @@
 			return configure(new FileEditorsPreferencePage());
 		}
 		if (KEYS_PREFERENCE_PAGE.equals(id)) {
-			return configure(new NoKeysPreferencePage());
+			return configure(new KeysPreferencePage());
 		}
 		if (NEW_KEYS_PREFERENCE_PAGE.equals(id)) {
-			return configure(new NoKeysPreferencePage());
+			return configure(new NewKeysPreferencePage());
 		}
 		if (PERSPECTIVES_PREFERENCE_PAGE.equals(id)) {
 			return configure(new PerspectivesPreferencePage());
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/BindingToModelProcessor.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/BindingToModelProcessor.java
index a119413..3e5fa44 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/BindingToModelProcessor.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/BindingToModelProcessor.java
@@ -19,6 +19,7 @@
 import org.eclipse.core.commands.ParameterizedCommand;
 import org.eclipse.core.commands.contexts.ContextManager;
 import org.eclipse.e4.core.di.annotations.Execute;
+import org.eclipse.e4.ui.bindings.internal.BindingCopies;
 import org.eclipse.e4.ui.model.application.MApplication;
 import org.eclipse.e4.ui.model.application.commands.MBindingContext;
 import org.eclipse.e4.ui.model.application.commands.MBindingTable;
@@ -78,6 +79,11 @@
 	}
 
 	public final void addBinding(final MApplication application, final Binding binding) {
+
+		if (binding.getType() == Binding.USER) {
+			BindingCopies.addUserBinding(binding);
+		}
+
 		MBindingTable table = tables.get(binding.getContextId());
 		if (table == null) {
 			table = createTable(application, binding.getContextId());
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/BindingPersistence.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/BindingPersistence.java
index 149bf65..64475e7c 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/BindingPersistence.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/BindingPersistence.java
@@ -35,6 +35,7 @@
 import org.eclipse.core.runtime.IExtensionRegistry;
 import org.eclipse.core.runtime.IRegistryChangeEvent;
 import org.eclipse.core.runtime.Platform;
+import org.eclipse.e4.ui.bindings.internal.BindingCopies;
 import org.eclipse.jface.bindings.Binding;
 import org.eclipse.jface.bindings.BindingManager;
 import org.eclipse.jface.bindings.Scheme;
@@ -480,6 +481,10 @@
 			final BindingManager bindingManager, final CommandManager commandService) {
 		List warningsToLog = new ArrayList(1);
 
+		// flag to deactivate a SYSTEM binding
+		boolean flagDeactivate = false;
+		int bindingType = Binding.USER;
+
 		if (preferences != null) {
 			final IMemento[] preferenceMementos = preferences
 					.getChildren(TAG_KEY_BINDING);
@@ -575,13 +580,42 @@
 							warningsToLog, command);
 				}
 
+				// see if this is actually a USER binding or a default SYSTEM
+				// binding that the user deactivated
+				String isDeactivatedSystemBinding = readOptional(memento, ATT_DELETED);
+				if (isDeactivatedSystemBinding != null && isDeactivatedSystemBinding.equals("true")) { //$NON-NLS-1$
+					flagDeactivate = true;
+				}
+
+				if (flagDeactivate) {
+					bindingType = Binding.SYSTEM;
+				} else {
+					bindingType = Binding.USER;
+				}
+
 				final Binding binding = new KeyBinding(keySequence,
-						parameterizedCommand, schemeId, contextId, locale,
-						platform, null, Binding.USER);
-				bindingManager.addBinding(binding);
+						parameterizedCommand, schemeId, contextId, locale, platform, null, bindingType);
+
+				if (flagDeactivate) {
+					BindingCopies.addInactiveSysBinding(binding);
+				} else {
+					bindingManager.addBinding(binding);
+				}
+
+				// reset flag
+				flagDeactivate = false;
 			}
 		}
 
+		Binding[] sysBindingsToRemove = BindingCopies.getInactiveSysBindings();
+		for (int i = 0; i < sysBindingsToRemove.length; i++) {
+			// bindingManager.removeBinding(sysBindingsToRemove[i]);
+			bindingManager.removeBindings(sysBindingsToRemove[i].getTriggerSequence(),
+					sysBindingsToRemove[i].getSchemeId(), sysBindingsToRemove[i].getContextId(),
+					sysBindingsToRemove[i].getLocale(), sysBindingsToRemove[i].getPlatform(), null,
+					sysBindingsToRemove[i].getType());
+		}
+
 		// If there were any warnings, then log them now.
 		logWarnings(warningsToLog,
 				"Warnings while parsing the key bindings from the preference store"); //$NON-NLS-1$
@@ -1063,14 +1097,15 @@
 	 * 
 	 * @param activeScheme
 	 *            The scheme which should be persisted; may be <code>null</code>.
-	 * @param bindings
+	 * @param activeUserBindings
 	 *            The bindings which should be persisted; may be
 	 *            <code>null</code>
 	 * @throws IOException
 	 *             If something happens while trying to write to the workbench
 	 *             preference store.
 	 */
-	static final void write(final Scheme activeScheme, final Binding[] bindings)
+	static final void write(final Scheme activeScheme, final Binding[] activeUserBindings,
+			Binding[] inactiveSystemBindings)
 			throws IOException {
 		// Print out debugging information, if requested.
 		if (DEBUG) {
@@ -1088,12 +1123,23 @@
 		if (activeScheme != null) {
 			writeActiveSchemeToPreferences(xmlMemento, activeScheme);
 		}
-		if (bindings != null) {
-			final int bindingsLength = bindings.length;
+		// write all active USER Bindings
+		if (activeUserBindings != null) {
+			final int bindingsLength = activeUserBindings.length;
 			for (int i = 0; i < bindingsLength; i++) {
-				final Binding binding = bindings[i];
-				if (binding.getType() == Binding.USER) {
-					writeBindingToPreferences(xmlMemento, binding);
+				final Binding binding = activeUserBindings[i];
+				if (binding.getParameterizedCommand() != null && binding.getType() == Binding.USER) {
+					writeBindingToPreferences(xmlMemento, binding, false);
+				}
+			}
+		}
+		// write all deactivated SYSTEM bindings so that we know which default
+		// bindings to disable on the next startup
+		if (inactiveSystemBindings != null) {
+			for (int i = 0; i < inactiveSystemBindings.length; i++) {
+				final Binding inactiveSystemBinding = inactiveSystemBindings[i];
+				if (inactiveSystemBinding.getType() == Binding.SYSTEM) {
+					writeBindingToPreferences(xmlMemento, inactiveSystemBinding, true);
 				}
 			}
 		}
@@ -1170,7 +1216,7 @@
 	 *            The binding to write; must not be <code>null</code>.
 	 */
 	private static final void writeBindingToPreferences(final IMemento parent,
-			final Binding binding) {
+			final Binding binding, boolean inactiveSystemBinding) {
 		final IMemento element = parent.createChild(TAG_KEY_BINDING);
 		element.putString(ATT_CONTEXT_ID, binding.getContextId());
 		final ParameterizedCommand parameterizedCommand = binding
@@ -1183,6 +1229,11 @@
 				.toString());
 		element.putString(ATT_LOCALE, binding.getLocale());
 		element.putString(ATT_PLATFORM, binding.getPlatform());
+		if (inactiveSystemBinding) {
+			element.putString(ATT_DELETED, "true"); //$NON-NLS-1$
+		} else {
+			element.putString(ATT_DELETED, "false"); //$NON-NLS-1$
+		}
 		if (parameterizedCommand != null) {
 			final Map parameterizations = parameterizedCommand
 					.getParameterMap();
@@ -1226,6 +1277,7 @@
 			final CommandManager commandManager) {
 		this.bindingManager = bindingManager;
 		this.commandManager = commandManager;
+		BindingCopies.init();
 	}
 
 	protected final boolean isChangeImportant(final IRegistryChangeEvent event) {
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/BindingService.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/BindingService.java
index e9c48ce6..82fb3f0 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/BindingService.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/BindingService.java
@@ -18,10 +18,13 @@
 import java.util.List;
 import java.util.Map;
 import javax.inject.Inject;
+import org.eclipse.core.commands.CommandManager;
 import org.eclipse.core.commands.ParameterizedCommand;
 import org.eclipse.e4.core.commands.ECommandService;
 import org.eclipse.e4.core.di.annotations.Optional;
 import org.eclipse.e4.ui.bindings.EBindingService;
+import org.eclipse.e4.ui.bindings.internal.BindingCopies;
+import org.eclipse.e4.ui.bindings.internal.BindingTableManager;
 import org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher;
 import org.eclipse.e4.ui.model.application.MApplication;
 import org.eclipse.e4.ui.model.application.commands.MBindingContext;
@@ -67,6 +70,12 @@
 	private BindingManager manager;
 
 	@Inject
+	private CommandManager cmdManager;
+
+	@Inject
+	private BindingTableManager bindingTableManager;
+
+	@Inject
 	@Optional
 	private KeyBindingDispatcher dispatcher;
 
@@ -338,6 +347,16 @@
 	 * .ui.commands.ICommandService)
 	 */
 	public void readRegistryAndPreferences(ICommandService commandService) {
+
+
+		// TODO: shouldn't be using BindingPersistence here, but if we don't,
+		// then the keys pref page will crash when loading!
+		BindingPersistence bindingPersistence = new BindingPersistence(manager, cmdManager);
+		bindingPersistence.reRead();
+
+		Collection<Binding> bindings = bindingTableManager.getActiveBindings();
+		manager.setBindings(bindings.toArray(new Binding[bindings.size()]));
+
 		// BindingPersistence reader = new BindingPersistence(manager,
 		// commandService);
 		// reader.reRead();
@@ -347,6 +366,8 @@
 		// Binding binding = (Binding) i.next();
 		// addBinding(binding);
 		// }
+
+		bindingPersistence.dispose();
 	}
 
 	private MCommand findCommand(String id) {
@@ -366,8 +387,107 @@
 	 * .bindings.Scheme, org.eclipse.jface.bindings.Binding[])
 	 */
 	public void savePreferences(Scheme activeScheme, Binding[] bindings) throws IOException {
-		// TODO compat savePreferences
-		E4Util.unsupported("savePreferences"); //$NON-NLS-1$
+
+		Collection<Binding> activeBindings = bindingTableManager.getActiveBindings();
+		Binding[] activeBindingsArray = activeBindings.toArray(new Binding[activeBindings.size()]);
+
+		Binding b;
+
+		Collection<Binding> activeUserBindings = new ArrayList<Binding>();
+
+		boolean madeChanges = false; // flag to actually save the prefs
+
+		// go through the list of active bindings (from the BTM) and DEACTIVATE
+		// any bindings that are not included in the list of bindings passed
+		// from the new keys pref page
+		for (int i = 0; i < activeBindingsArray.length; i++) {
+			b = getEqualBinding(bindings, activeBindingsArray[i]);
+
+			// if we didn't find the binding, then it wasn't included in the
+			// list passed from the new keys pref page, so this binding needs to
+			// be deactivated
+			if (b == null) {
+				bindingService.deactivateBinding(activeBindingsArray[i]);
+
+				// make sure we keep this binding manager consistent with the
+				// BTM since the keys pref page will read off this
+				manager.removeBinding(activeBindingsArray[i]);
+
+				// if we're deactivating a SYSTEM binding, then keep it in the
+				// list of bindings to write to xml, but make sure it gets
+				// flagged as disabled
+				if (activeBindingsArray[i].getType() == Binding.SYSTEM) {
+					// inactiveSystemBindings.add(activeBindingsArray[i]);
+					BindingCopies.addInactiveSysBinding(activeBindingsArray[i]);
+				}
+
+				// flag that changes were made to the bindings so that we need
+				// to save
+				madeChanges = true;
+			}
+		}
+
+		// go though the list of bindings passed from the new keys pref page and
+		// ACTIVATE the NEW bindings added
+		for (int i = 0; i < bindings.length; i++) {
+			b = getEqualBinding(activeBindingsArray, bindings[i]);
+
+			// if we didn't find the binding, that means it's a new one, so
+			// let's activate it (as long as the command isn't null)
+			if (b == null && bindings[i].getParameterizedCommand() != null) {
+
+				bindingService.activateBinding(bindings[i]);
+
+				// make sure we keep this binding manager consistent with the
+				// BTM since the keys pref page will read off this
+				manager.addBinding(bindings[i]);
+
+				// flag that changes were made to the bindings, so we need
+				// to persist them
+				madeChanges = true;
+
+				// Since the binding persistence only writes out active USER
+				// bindings and deactivated SYSTEM bindings, there's no point in
+				// sending the entire bindings list and searching USER bindings.
+				if (bindings[i].getType() == Binding.USER) {
+					activeUserBindings.add(bindings[i]);
+				} else {
+					BindingCopies.removeInactiveSysBinding(bindings[i]);
+				}
+
+				// since we've activated a new binding, we need to see if it's
+				// replacing one of the old bindings (in which case the old
+				// binding needs to be deactivated)
+				// ... but not right now
+			}
+			// else if the binding was already activated AND it's a USER
+			// binding, then we'll need to persist it as well
+			else if (bindings[i].getType() == Binding.USER) {
+				activeUserBindings.add(bindings[i]);
+				madeChanges = true;
+			}
+		}
+
+		// if we made any changes, then they need to be persisted
+		if (madeChanges) {
+			// BindingPersistence.write(activeScheme, bindings);
+			BindingPersistence.write(activeScheme,
+					activeUserBindings.toArray(new Binding[activeUserBindings.size()]),
+					BindingCopies.getInactiveSysBindings());
+			// TODO: these numbers don't make sense... dig a litter deeper
+		}
+
+	}
+
+	private Binding getEqualBinding(Binding[] bindings, Binding target) {
+		Binding theBinding = null;
+
+		for (int i = 0; i < bindings.length && theBinding == null; i++) {
+			if (bindings[i].equals(target)) {
+				theBinding = bindings[i];
+			}
+		}
+		return theBinding;
 	}
 
 	/*
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/model/KeyController.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/model/KeyController.java
index dd71efc..70ef47b 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/model/KeyController.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/keys/model/KeyController.java
@@ -16,6 +16,7 @@
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
+import java.util.Collection;
 import java.util.Map;
 import java.util.ResourceBundle;
 import org.eclipse.core.commands.CommandManager;
@@ -26,6 +27,7 @@
 import org.eclipse.core.runtime.ListenerList;
 import org.eclipse.core.runtime.SafeRunner;
 import org.eclipse.core.runtime.Status;
+import org.eclipse.e4.ui.bindings.internal.BindingCopies;
 import org.eclipse.jface.bindings.Binding;
 import org.eclipse.jface.bindings.BindingManager;
 import org.eclipse.jface.bindings.Scheme;
@@ -379,16 +381,6 @@
 							activeBinding);
 
 					// Remove binding for any system conflicts
-					Object[] keys = bindingToElement.keySet().toArray();
-					for (int i = 0; i < keys.length; i++) {
-						Binding bindingKey = (Binding) keys[i];
-						if (oldSequence.equals(bindingKey.getTriggerSequence())
-								&& bindingKey.getType() == Binding.SYSTEM) {
-							BindingElement be = (BindingElement) bindingToElement
-									.get(bindingKey);
-							bindingModel.remove(be);
-						}
-					}
 
 					bindingModel.setSelectedElement(activeBinding);
 				} else {
@@ -433,8 +425,15 @@
 	 */
 	public void saveBindings(IBindingService bindingService) {
 		try {
-			bindingService.savePreferences(fBindingManager.getActiveScheme(),
-					fBindingManager.getBindings());
+
+			Collection<Binding> activeBindingsCollection = fBindingManager
+					.getActiveBindingsDisregardingContextFlat();
+			Binding[] activeBindingsArray = activeBindingsCollection
+					.toArray(new Binding[activeBindingsCollection.size()]);
+
+			bindingService.savePreferences(fBindingManager.getActiveScheme(), activeBindingsArray);
+			// bindingService.savePreferences(fBindingManager.getActiveScheme(),fBindingManager.getBindings());
+
 		} catch (IOException e) {
 			logPreferenceStoreException(e);
 		}
@@ -486,6 +485,7 @@
 			fBindingManager.setActiveScheme(defaultScheme);
 		} catch (final NotDefinedException e) {
 			// At least we tried....
+			e.printStackTrace();
 		}
 
 		// Restore any User defined bindings
@@ -495,6 +495,18 @@
 				fBindingManager.removeBinding(bindings[i]);
 			}
 		}
+		// Re-add the deactivated SYSTEM bindings
+		bindings = BindingCopies.getInactiveSysBindings();
+		for (int i = 0; i < bindings.length; i++) {
+			fBindingManager.addBinding(bindings[i]);
+		}
+		// reset the binding copies
+		BindingCopies.init();
+
+		// set the binding manager's bindings array our copy of the system
+		// bindings
+		// fBindingManager.setBindings(BindingCopies.getSystemBindings().toArray(
+		// new Binding[BindingCopies.getSystemBindings().size()]));
 
 		bindingModel.refresh(contextModel);
 		saveBindings(bindingService);
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/registry/IWorkbenchRegistryConstants.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/registry/IWorkbenchRegistryConstants.java
index 90123e0..2795090 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/registry/IWorkbenchRegistryConstants.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/registry/IWorkbenchRegistryConstants.java
@@ -156,6 +156,11 @@
 	public static String ATT_DEFINITION_ID = "definitionId";//$NON-NLS-1$
 
 	/**
+	 * Resembles a deactivated SYSTEM binding. Value <code>deleted</code>.
+	 */
+	public static String ATT_DELETED = "deleted";//$NON-NLS-1$	
+
+	/**
 	 * The name of the description attribute, which appears on named handle
 	 * objects.
 	 */
diff --git a/tests/org.eclipse.e4.ui.bindings.tests/src/org/eclipse/e4/ui/bindings/tests/BindingLookupTest.java b/tests/org.eclipse.e4.ui.bindings.tests/src/org/eclipse/e4/ui/bindings/tests/BindingLookupTest.java
index ecad5cf..633fba6 100644
--- a/tests/org.eclipse.e4.ui.bindings.tests/src/org/eclipse/e4/ui/bindings/tests/BindingLookupTest.java
+++ b/tests/org.eclipse.e4.ui.bindings.tests/src/org/eclipse/e4/ui/bindings/tests/BindingLookupTest.java
@@ -473,9 +473,8 @@
 	private Binding createDefaultBinding(EBindingService bs,
 			TriggerSequence sequence, ParameterizedCommand command,
 			String contextId) {
-		return bs.createBinding(sequence, command,
-				"org.eclipse.ui.defaultAcceleratorConfiguration", //$NON-NLS-1$
-				contextId);
+		return bs.createBinding(sequence, command, "org.eclipse.ui.defaultAcceleratorConfiguration",
+				contextId, null, null, Binding.SYSTEM);
 	}
 
 }
diff --git a/tests/org.eclipse.e4.ui.bindings.tests/src/org/eclipse/e4/ui/bindings/tests/KeyDispatcherTest.java b/tests/org.eclipse.e4.ui.bindings.tests/src/org/eclipse/e4/ui/bindings/tests/KeyDispatcherTest.java
index 2dd2422..65d9279 100644
--- a/tests/org.eclipse.e4.ui.bindings.tests/src/org/eclipse/e4/ui/bindings/tests/KeyDispatcherTest.java
+++ b/tests/org.eclipse.e4.ui.bindings.tests/src/org/eclipse/e4/ui/bindings/tests/KeyDispatcherTest.java
@@ -98,9 +98,7 @@
 
 	private Binding createDefaultBinding(EBindingService bs,
 			TriggerSequence sequence, ParameterizedCommand command) {
-		return bs.createBinding(sequence, command,
-				"org.eclipse.ui.defaultAcceleratorConfiguration", //$NON-NLS-1$
-				ID_WINDOW);
+		return bs.createBinding(sequence, command, "org.eclipse.ui.defaultAcceleratorConfiguration", ID_WINDOW, null, null, Binding.SYSTEM);
 	}
 
 	@Override