Bug 522286: [9] Refresh Classpath and Module path elements if
non-modular element is made modular or vice versa

- polish

Change-Id: Ia0228843e6942cd5edd80fbf8fb397dae30d8819
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/NewWizardMessages.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/NewWizardMessages.java
index 3325ee9..7f1af3c 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/NewWizardMessages.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/NewWizardMessages.java
@@ -461,6 +461,7 @@
 	public static String ModuleDialog_removeFromIncluded_tooltip;
 	public static String ModuleDialog_addToExplicitlyIncluded_tooltip;
 	public static String ModuleDialog_cannotLimitSingle_error;
+	public static String ModuleDialog_unknownModules_info;
 
 	public static String ModuleDialog_details_tab;
 	public static String ModuleDialog_patches_module_label;
@@ -480,6 +481,7 @@
 	public static String ModuleDialog_duplicateReads_error;
 	public static String ModuleDialog_missingPatch_error;
 	public static String ModuleDialog_wrongPatch_error;
+	public static String ModuleDialog_cannotEditDetails_info;
 	public static String ModuleDialog_mustIncludeModule_error;
 
 	public static String ModuleAddExportsBlock_packageEmpty_error;
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/NewWizardMessages.properties b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/NewWizardMessages.properties
index 6bb3cc5..0c4e518 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/NewWizardMessages.properties
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/NewWizardMessages.properties
@@ -498,6 +498,7 @@
 ModuleDialog_removeFromIncluded_tooltip=Remove selected from included modules
 ModuleDialog_addToExplicitlyIncluded_tooltip=Add selected to explicitly included modules
 ModuleDialog_cannotLimitSingle_error=Cannot limit a single-module entry
+ModuleDialog_unknownModules_info=Contained modules are not known until changes are applied.
 
 ModuleDialog_details_tab=Details
 ModuleDialog_patches_module_label=Patches an existing module
@@ -518,6 +519,7 @@
 ModuleDialog_missingPatch_error=Must specify the module to be patched
 ModuleDialog_wrongPatch_error=Module ''{0}'' is not provided by this build path entry
 ModuleDialog_mustIncludeModule_error=Must include at least one module
+ModuleDialog_cannotEditDetails_info=Most details can only be edited after changes have been applied
 
 ModuleAddExportsBlock_packageEmpty_error=Package is empty
 ModuleAddExportsBlock_sourceModuleEmpty_error=Source module is empty
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/BuildPathBasePage.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/BuildPathBasePage.java
index 85f8264..3809acf 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/BuildPathBasePage.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/BuildPathBasePage.java
@@ -28,6 +28,7 @@
 import org.eclipse.core.resources.IWorkspaceRunnable;
 
 import org.eclipse.jface.dialogs.MessageDialog;
+import org.eclipse.jface.viewers.StructuredSelection;
 import org.eclipse.jface.window.Window;
 
 import org.eclipse.ui.PlatformUI;
@@ -47,6 +48,7 @@
 import org.eclipse.jdt.internal.ui.preferences.PreferencesMessages;
 import org.eclipse.jdt.internal.ui.util.ExceptionHandler;
 import org.eclipse.jdt.internal.ui.wizards.NewWizardMessages;
+import org.eclipse.jdt.internal.ui.wizards.buildpaths.RootCPListElement.RootNodeChange;
 import org.eclipse.jdt.internal.ui.wizards.dialogfields.IDialogFieldListener;
 import org.eclipse.jdt.internal.ui.wizards.dialogfields.ITreeListAdapter;
 import org.eclipse.jdt.internal.ui.wizards.dialogfields.TreeListDialogField;
@@ -278,6 +280,46 @@
 	
 
 
+	/**
+	 * @param listField the UI element holding the list of elements to be manipulated
+	 * @param selElement is classpath element
+	 * @param changeNodeDirection indicate in which direction the element should be moved
+	 */
+	protected void moveCPElementAcrossNode(TreeListDialogField<CPListElement> listField, CPListElement selElement, RootNodeChange changeNodeDirection) {
+		List<CPListElement> elements= listField.getElements();
+		//remove from module node or classnode
+		for (CPListElement cpListElement : elements) {
+			if (cpListElement.isRootNodeForPath()) {
+				RootCPListElement rootElement= (RootCPListElement) cpListElement;
+				if (rootElement.isSourceRootNode(changeNodeDirection) && rootElement.getChildren().contains(selElement)) {
+					rootElement.removeCPListElement(selElement);
+					listField.getTreeViewer().remove(selElement);
+					listField.dialogFieldChanged();
+				}
+			}
+		}
+		// add to classpath node or module and select the cpe
+		for (CPListElement cpListElement : elements) {
+			if (cpListElement.isRootNodeForPath()) {
+				RootCPListElement rootCPListElement= (RootCPListElement) cpListElement;
+				if (rootCPListElement.isTargetRootNode(changeNodeDirection)) {
+					if (rootCPListElement.getChildren().contains(selElement))
+						break;
+					rootCPListElement.addCPListElement(selElement);
+					List<CPListElement> all= listField.getElements();
+					listField.removeAllElements();
+					listField.setElements(all);
+					listField.refresh();
+					listField.getTreeViewer().expandToLevel(2);
+					listField.postSetSelection(new StructuredSelection(selElement));
+					break;
+				}
+			}
+		}
+	}
+
+
+
 	protected abstract class CPListAdapter implements IDialogFieldListener, ITreeListAdapter<CPListElement> {
 		private final Object[] EMPTY_ARR= new Object[0];
 
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/LibrariesWorkbookPage.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/LibrariesWorkbookPage.java
index 9afee7f..062cd96 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/LibrariesWorkbookPage.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/LibrariesWorkbookPage.java
@@ -75,6 +75,7 @@
 import org.eclipse.jdt.internal.ui.jarimport.JarImportWizard;
 import org.eclipse.jdt.internal.ui.util.ExceptionHandler;
 import org.eclipse.jdt.internal.ui.wizards.NewWizardMessages;
+import org.eclipse.jdt.internal.ui.wizards.buildpaths.RootCPListElement.RootNodeChange;
 import org.eclipse.jdt.internal.ui.wizards.dialogfields.CheckedListDialogField;
 import org.eclipse.jdt.internal.ui.wizards.dialogfields.DialogField;
 import org.eclipse.jdt.internal.ui.wizards.dialogfields.LayoutUtil;
@@ -170,8 +171,8 @@
 		fLibrariesList.setElements(libelements);
 	}
 	private void updateLibrariesListWithRootNode() {
-		CPListElement rootClasspath = new RootCPListElement(fCurrJProject, NewWizardMessages.PathRootWorkbookPage_classpath,false);
-		CPListElement rootModulepath = new RootCPListElement(fCurrJProject,NewWizardMessages.PathRootWorkbookPage_modulepath,true);
+		RootCPListElement rootClasspath= new RootCPListElement(fCurrJProject, NewWizardMessages.PathRootWorkbookPage_classpath,false);
+		RootCPListElement rootModulepath= new RootCPListElement(fCurrJProject,NewWizardMessages.PathRootWorkbookPage_modulepath,true);
 
 		List<CPListElement> cpelements= fClassPathList.getElements();
 		List<CPListElement> libelements= new ArrayList<>(cpelements.size());
@@ -180,11 +181,11 @@
 		for (int i= 0; i < nElements; i++) {
 			CPListElement cpe= cpelements.get(i);
 			if (isEntryKind(cpe.getEntryKind())) {
-				Object mod = cpe.getAttribute(CPListElement.MODULE);
-				if(mod==null) {
-					((RootCPListElement)rootClasspath).addCPListElement(cpe);
+				Object mod= cpe.getAttribute(CPListElement.MODULE);
+				if(mod == null) {
+					rootClasspath.addCPListElement(cpe);
 				} else {
-					((RootCPListElement)rootModulepath).addCPListElement(cpe);
+					rootModulepath.addCPListElement(cpe);
 				}
 					
 			}
@@ -327,9 +328,9 @@
 
 			for (int i= 0; i < nElementsChosen; i++) {
 				CPListElement curr= libentries[i];
-				boolean contains = cplist.contains(curr);
+				boolean contains= cplist.contains(curr);
 				if(hasRootNodes()) {
-					contains = hasCurrentElement(cplist,curr);
+					contains= hasCurrentElement(cplist,curr);
 				}
 				if (!contains && !elementsToAdd.contains(curr)) {
 					elementsToAdd.add(curr);
@@ -343,32 +344,29 @@
 			
 			if(!hasRootNodes()) {
 				fLibrariesList.addElements(elementsToAdd);
-			}
-			else {
-				// on the new nodes, only additions allowed, rest disabled
+			} else {
+				// on root nodes, only additions allowed, rest disabled
 				List<Object> selectedElements= fLibrariesList.getSelectedElements();
 				List<CPListElement> elements= fLibrariesList.getElements();
-				// if nothing selected, do nothing
-				if(selectedElements.size()==0) {
+				// sanity check, button should only be enabled if exactly one root node is selected 
+				if(selectedElements.size() != 1) {
 					return;
 				}
 				fLibrariesList.removeAllElements();
-				for (int i= 0; i < selectedElements.size(); i++) {
-					if( ((CPListElement)selectedElements.get(i)).isClassPathRootNode()) {
-						for (CPListElement cpListElement : elementsToAdd) {
-							cpListElement.setAttribute(IClasspathAttribute.MODULE, null);
+				RootCPListElement selectedCPElement= (RootCPListElement) selectedElements.get(0);
+				if(selectedCPElement.isClassPathRootNode()) {
+					for (CPListElement cpListElement : elementsToAdd) {
+						cpListElement.setAttribute(IClasspathAttribute.MODULE, null);
+					}
+				} else if(selectedCPElement.isModulePathRootNode()) {
+					for (CPListElement cpListElement : elementsToAdd) {
+						Object attribute= cpListElement.getAttribute(IClasspathAttribute.MODULE);
+						if(attribute == null) {
+							cpListElement.setAttribute(IClasspathAttribute.MODULE, new ModuleEncapsulationDetail[0]);
 						}
 					}
-					if( ((CPListElement)selectedElements.get(i)).isModulePathRootNode()) {
-						for (CPListElement cpListElement : elementsToAdd) {
-							Object attribute= cpListElement.getAttribute(IClasspathAttribute.MODULE);
-							if(attribute == null) {
-								cpListElement.setAttribute(IClasspathAttribute.MODULE, new ModuleEncapsulationDetail[0]);
-							}
-						}
-					}
-					((RootCPListElement)selectedElements.get(i)).addCPListElement(elementsToAdd);					
 				}
+				selectedCPElement.addCPListElement(elementsToAdd);					
 				
 				fLibrariesList.setElements(elements);
 				fLibrariesList.refresh();
@@ -386,7 +384,7 @@
 		//note that the same cpelement with different attribute can be added
 		for (CPListElement cpListElement : cplist) {
 			if(cpListElement.isRootNodeForPath()) {
-				boolean cont =( (RootCPListElement)cpListElement).getChildren().contains(curr);
+				boolean cont= ( (RootCPListElement)cpListElement).getChildren().contains(curr);
 				if(cont == true) {
 					return true;
 				}
@@ -586,9 +584,9 @@
 		}
 		Object elem= selElements.get(0);
 		
-		boolean canEdit = false;
+		boolean canEdit= false;
 		if(hasRootNodes()) {
-			canEdit = ((RootCPListElement)fLibrariesList.getElement(0)).getChildren().indexOf(elem) != -1 || 
+			canEdit= ((RootCPListElement)fLibrariesList.getElement(0)).getChildren().indexOf(elem) != -1 || 
 					((RootCPListElement)fLibrariesList.getElement(1)).getChildren().indexOf(elem) != -1 ;
 			
 		}
@@ -639,13 +637,16 @@
 				}
 			}
 		} else if (key.equals(CPListElement.MODULE)) {
+			boolean wasModular= selElement.getAttribute(CPListElement.MODULE) != null;
 			if (showModuleDialog(getShell(), elem)) {
 				String[] changedAttributes= { CPListElement.MODULE };
 				attributeUpdated(selElement, changedAttributes);
 				if (hasRootNodes()) {
-					Object mod= selElement.getAttribute(CPListElement.MODULE);
-					boolean remove= mod == null;
-					moveCPElementAcrossNode(selElement, remove);
+					boolean isModular= selElement.getAttribute(CPListElement.MODULE) != null;
+					RootNodeChange direction= RootNodeChange.fromOldAndNew(wasModular, isModular);
+					if (direction != RootNodeChange.NoChange) {
+						moveCPElementAcrossNode(fLibrariesList, selElement, direction);
+					}
 				}
 				fLibrariesList.refresh(elem);
 				fClassPathList.dialogFieldChanged(); // validate
@@ -744,9 +745,16 @@
 								Object obj= ((RootCPListElement) cpe).getChildren().get(j);
 								if (obj.equals(elem)) {
 									((RootCPListElement) cpe).getChildren().set(j, curr);
-									int changeNodeLocation= doesElementNeedNodeChange(elem, curr);
-									if (changeNodeLocation != 0) {
-										moveCPElementAcrossNode(curr, changeNodeLocation == -1);
+									RootNodeChange changeNodeDirection= doesElementNeedNodeChange(elem, curr);
+									if (changeNodeDirection != RootNodeChange.NoChange) {
+										moveCPElementAcrossNode(fLibrariesList, curr, changeNodeDirection);
+										CPListElementAttribute moduleAttr= curr.findAttributeElement(CPListElement.MODULE);
+										Object value=  (changeNodeDirection == RootNodeChange.ToModulepath) ? new ModuleEncapsulationDetail[0] : null;
+										if (moduleAttr != null) {
+											moduleAttr.setValue(value);
+										} else {
+											curr.setAttribute(CPListElement.MODULE, value);
+										}
 									}
 									fLibrariesList.dialogFieldChanged();
 									fLibrariesList.refresh();
@@ -767,56 +775,16 @@
 	}
 
 	/**
-	 * @param curr is classpath element
-	 * @param remove if true element needs to be removed from modulepath ( if present) and put in
-	 *            classpath and if false element need to be moved from classpath (if present) to
-	 *            modulepath
-	 */
-	private void moveCPElementAcrossNode(CPListElement curr, boolean remove) {
-		List<CPListElement> elements= fLibrariesList.getElements();
-		CPListElement selElement= curr;
-		//remove from module node or classnode
-		for (CPListElement cpListElement : elements) {
-			if (cpListElement.isRootNodeForPath()) {
-				if ((remove ? cpListElement.isModulePathRootNode() : cpListElement.isClassPathRootNode()) && ((RootCPListElement) cpListElement).getChildren().contains(selElement)) {
-					((RootCPListElement) cpListElement).removeCPListElement(selElement);
-					fLibrariesList.getTreeViewer().remove(selElement);
-					fLibrariesList.dialogFieldChanged();
-				}
-			}
-		}
-		// add to classpath node or module and select the cpe
-		for (CPListElement cpListElement : elements) {
-			if (cpListElement.isRootNodeForPath()) {
-				if (remove ? cpListElement.isClassPathRootNode() : cpListElement.isModulePathRootNode()) {
-					RootCPListElement rootCPListElement= (RootCPListElement) cpListElement;
-					if (rootCPListElement.getChildren().contains(selElement))
-						break;
-					rootCPListElement.addCPListElement(selElement);
-					List<CPListElement> all= fLibrariesList.getElements();
-					fLibrariesList.removeAllElements();
-					fLibrariesList.setElements(all);
-					fLibrariesList.refresh();
-					fLibrariesList.getTreeViewer().expandToLevel(2);
-					fLibrariesList.postSetSelection(new StructuredSelection(selElement));
-					break;
-				}
-			}
-		}
-
-	}
-
-	/**
 	 * @param elem is former cpelement
 	 * @param curr is new cpelement
 	 * @return 0 is no change is required, -1 if modular element is removed, +1 if non-modular element
 	 *         is removed.
 	 */
-	private int doesElementNeedNodeChange(CPListElement elem, CPListElement curr) {
+	private RootNodeChange doesElementNeedNodeChange(CPListElement elem, CPListElement curr) {
 		if (elem.getClasspathEntry().getEntryKind() != IClasspathEntry.CPE_CONTAINER)
-			return 0;
+			return RootNodeChange.NoChange;
 		if (curr.getClasspathEntry().getEntryKind() != IClasspathEntry.CPE_CONTAINER)
-			return 0;
+			return RootNodeChange.NoChange;
 		String v1= null;
 		String v2= null;
 		IVMInstall vm1= JavaRuntime.getVMInstall(elem.getPath());
@@ -831,19 +799,19 @@
 			boolean mod1= JavaModelUtil.is9OrHigher(v1);
 			boolean mod2= JavaModelUtil.is9OrHigher(v2);
 			if (mod1 == true && mod2 == true)
-				return 0;
+				return RootNodeChange.NoChange;
 			if (mod1 == false && mod2 == false)
-				return 0;
+				return RootNodeChange.NoChange;
 			if (mod1 == true && mod2 == false) {
 				// removed module
-				return -1;
+				return RootNodeChange.ToClasspath;
 			}
 			if (mod1 == false && mod2 == true) {
 				//added module
-				return 1;
+				return RootNodeChange.ToModulepath;
 			}
 		}
-		return 0;
+		return RootNodeChange.NoChange;
 	}
 
 	/**
@@ -931,9 +899,9 @@
 
 	private void updateClasspathList() {
 		 List<CPListElement> projelements= fLibrariesList.getElements();
-		 List<CPListElement> flattenedProjElements = new ArrayList<>();
+		 List<CPListElement> flattenedProjElements= new ArrayList<>();
 		 for ( int i =0; i < projelements.size(); i++ ) {
-		 	CPListElement ele = projelements.get(i);
+		 	CPListElement ele= projelements.get(i);
 		 	// if root node, collect the CPList elements
 		 	if(ele.isRootNodeForPath()) {
 		 		ArrayList<Object> children= ((RootCPListElement)ele).getChildren();
@@ -1150,7 +1118,7 @@
 			IPath[] paths= BuildPathDialogAccess.chooseVariableEntries(getShell(), existingPathsArray);
 			if (paths != null) {
 				ArrayList<CPListElement> result= new ArrayList<>();
-				for (int i = 0; i < paths.length; i++) {
+				for (int i= 0; i < paths.length; i++) {
 					IPath path= paths[i];
 					CPListElement elem= createCPVariableElement(path);
 					if (!existingElements.contains(elem)) {
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/ModuleDialog.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/ModuleDialog.java
index 0d1ab2f..66eaa81 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/ModuleDialog.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/ModuleDialog.java
@@ -545,7 +545,7 @@
 	protected void doSelectionChanged(ListDialogField<? extends ModuleEncapsulationDetail> field) {
 		boolean isModular= fIsModuleCheckbox.isSelected();
 		List<? extends ModuleEncapsulationDetail> selected= field.getSelectedElements();
-		field.enableButton(IDX_ADD, isModular);
+		field.enableButton(IDX_ADD, isModular && fJavaElements != null);
 		field.enableButton(IDX_EDIT, isModular && canEdit(selected));
 		field.enableButton(IDX_REMOVE, isModular && selected.size() > 0);
 		validateDetails(field);
@@ -587,10 +587,14 @@
 	private IStatus computeContentsStatus() {
 		StatusInfo info= new StatusInfo();
 		if (fIsModuleCheckbox.isSelected()) {
-			if (fModuleLists[IDX_INCLUDED].fNames.isEmpty()) {
-				info.setError(NewWizardMessages.ModuleDialog_mustIncludeModule_error);
-			} else if (fModuleLists[IDX_INCLUDED].fNames.size() + fModuleLists[IDX_AVAILABLE].fNames.size() == 1) {
-				info.setInfo(NewWizardMessages.ModuleDialog_cannotLimitSingle_error);
+			if (fJavaElements != null) {
+				if (fModuleLists[IDX_INCLUDED].fNames.isEmpty()) {
+					info.setError(NewWizardMessages.ModuleDialog_mustIncludeModule_error);
+				} else if (fModuleLists[IDX_INCLUDED].fNames.size() + fModuleLists[IDX_AVAILABLE].fNames.size() == 1) {
+					info.setInfo(NewWizardMessages.ModuleDialog_cannotLimitSingle_error);
+				}
+			} else {
+				info.setInfo(NewWizardMessages.ModuleDialog_unknownModules_info);
 			}
 		}
 		return info;
@@ -653,6 +657,9 @@
 				}
 			}
 		}
+		if (status.isOK() && fJavaElements == null) {
+			status.setInfo(NewWizardMessages.ModuleDialog_cannotEditDetails_info);
+		}
 		return status;
 	}
 
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/ProjectsWorkbookPage.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/ProjectsWorkbookPage.java
index f47c6e0..3fc6731 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/ProjectsWorkbookPage.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/ProjectsWorkbookPage.java
@@ -44,6 +44,7 @@
 import org.eclipse.jdt.internal.ui.JavaPlugin;
 import org.eclipse.jdt.internal.ui.viewsupport.JavaUILabelProvider;
 import org.eclipse.jdt.internal.ui.wizards.NewWizardMessages;
+import org.eclipse.jdt.internal.ui.wizards.buildpaths.RootCPListElement.RootNodeChange;
 import org.eclipse.jdt.internal.ui.wizards.dialogfields.DialogField;
 import org.eclipse.jdt.internal.ui.wizards.dialogfields.LayoutUtil;
 import org.eclipse.jdt.internal.ui.wizards.dialogfields.ListDialogField;
@@ -135,8 +136,8 @@
 		return false;
 	}
 	private void updateProjectsListWithRootNode() {
-		CPListElement rootClasspath = new RootCPListElement(fCurrJProject, NewWizardMessages.PathRootWorkbookPage_classpath,false);
-		CPListElement rootModulepath =  new RootCPListElement(fCurrJProject,NewWizardMessages.PathRootWorkbookPage_modulepath,true);
+		RootCPListElement rootClasspath = new RootCPListElement(fCurrJProject, NewWizardMessages.PathRootWorkbookPage_classpath,false);
+		RootCPListElement rootModulepath =  new RootCPListElement(fCurrJProject,NewWizardMessages.PathRootWorkbookPage_modulepath,true);
 
 		// add the projects-cpentries that are already on the class path
 		List<CPListElement> cpelements= fClassPathList.getElements();
@@ -148,9 +149,9 @@
 			if (isEntryKind(cpelem.getEntryKind())) {
 				Object mod= cpelem.getAttribute(CPListElement.MODULE);
 				if(mod == null) {
-					((RootCPListElement)rootClasspath).addCPListElement(cpelem);
+					rootClasspath.addCPListElement(cpelem);
 				} else {
-					((RootCPListElement)rootModulepath).addCPListElement(cpelem);
+					rootModulepath.addCPListElement(cpelem);
 				}
 			}
 		}
@@ -471,50 +472,24 @@
 	private void editAttributeEntry(CPListElementAttribute elem) {
 		String key= elem.getKey();
 		boolean needRefresh= false;
+		boolean wasModular= false;
 		if (key.equals(CPListElement.ACCESSRULES)) {
 			showAccessRestrictionDialog(elem.getParent());
 		} else if (key.equals(CPListElement.MODULE)) {
+			wasModular= elem.getValue() != null;
 			needRefresh= showModuleDialog(getShell(), elem);
 		} else {
 			needRefresh= editCustomAttribute(getShell(), elem);
 		}
 		if (needRefresh) {
 			if (key.equals(CPListElement.MODULE) && hasRootNodes()) {
-				// if module attribute is changed, the element 
-				// may change nodes
+				// if module attribute is changed, the element may change nodes
 				CPListElement selElement= elem.getParent();
-				Object mod= selElement.getAttribute(CPListElement.MODULE);
-				boolean remove= mod == null;
-				List<CPListElement> elements= fProjectsList.getElements();
-				//remove from module node or classnode
-				for (CPListElement cpListElement : elements) {
-					if (cpListElement.isRootNodeForPath()) {
-						if ((remove ? cpListElement.isModulePathRootNode() : cpListElement.isClassPathRootNode()) && ((RootCPListElement) cpListElement).getChildren().contains(selElement)) {
-							((RootCPListElement) cpListElement).removeCPListElement(selElement);
-							fProjectsList.getTreeViewer().remove(selElement);
-							fProjectsList.dialogFieldChanged();
-						}
-					}
+				boolean isModular= selElement.getAttribute(CPListElement.MODULE) != null;
+				RootNodeChange changeDirection= RootNodeChange.fromOldAndNew(wasModular, isModular);
+				if (changeDirection != RootNodeChange.NoChange) {
+					moveCPElementAcrossNode(fProjectsList, selElement, changeDirection);
 				}
-				// add to classpath node or module and select the cpe
-				for (CPListElement cpListElement : elements) {
-					if (cpListElement.isRootNodeForPath()) {
-						if (remove ? cpListElement.isClassPathRootNode() : cpListElement.isModulePathRootNode()) {
-							RootCPListElement rootCPListElement= (RootCPListElement) cpListElement;
-							if (rootCPListElement.getChildren().contains(cpListElement))
-								break;
-							rootCPListElement.addCPListElement(selElement);
-							List<CPListElement> all= fProjectsList.getElements();
-							fProjectsList.removeAllElements();
-							fProjectsList.setElements(all);
-							fProjectsList.refresh();
-							fProjectsList.getTreeViewer().expandToLevel(2);
-							fProjectsList.getTreeViewer().setSelection(new StructuredSelection(selElement));
-							break;
-						}
-					}
-				}
-
 			}
 			fProjectsList.refresh(elem);
 			fClassPathList.dialogFieldChanged(); // validate
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/RootCPListElement.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/RootCPListElement.java
index e99460c..74385c3 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/RootCPListElement.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/wizards/buildpaths/RootCPListElement.java
@@ -24,6 +24,20 @@
  */
 public class RootCPListElement extends CPListElement {
 
+	enum RootNodeChange {
+		ToModulepath, ToClasspath, NoChange;
+
+		static RootNodeChange fromOldAndNew(boolean wasModular, boolean isModular) {
+			if (wasModular == isModular) {
+				return NoChange;
+			} else if (isModular) {
+				return ToModulepath;
+			} else {
+				return ToClasspath;
+			}
+		}
+	}
+
 	private String fPathRootNodeName= null;
 
 	private boolean fIsModuleRootNode;
@@ -48,23 +62,15 @@
 	}
 
 	void addCPListElement(CPListElement cpe) {
-		if (isRootNodeForPath()) {
-			fChildren.add(cpe);
-		}
+		fChildren.add(cpe);
 	}
 
 	void addCPListElement(List<CPListElement> elementsToAdd) {
-		if (isRootNodeForPath()) {
-			fChildren.addAll(elementsToAdd);
-		}
+		fChildren.addAll(elementsToAdd);
 	}
 
 	ArrayList<Object> getChildren() {
-		if (isRootNodeForPath()) {
-			return fChildren;
-		} else {
-			return null;
-		}
+		return fChildren;
 	}
 
 	String getPathRootNodeName() {
@@ -86,9 +92,15 @@
 		return !fIsModuleRootNode;
 	}
 
+	public boolean isSourceRootNode(RootNodeChange changeNodeDirection) {
+		return RootNodeChange.fromOldAndNew(fIsModuleRootNode, !fIsModuleRootNode) == changeNodeDirection;
+	}
+
+	public boolean isTargetRootNode(RootNodeChange changeNodeDirection) {
+		return RootNodeChange.fromOldAndNew(!fIsModuleRootNode, fIsModuleRootNode) == changeNodeDirection;
+	}
+
 	void removeCPListElement(CPListElement element) {
-		if (isRootNodeForPath()) {
-			fChildren.remove(element);
-		}
+		fChildren.remove(element);
 	}
 }